Sample point distance estimate concern in sv_sp.py

Looking at SpatialVision's code in sv_sp.py, I have a concern about the method they have implemented for deriving distance to amenity estimates for sample points based on association with nearby nodes.
On line 219, there is the following code:
# for each sample point, create a new field to save the osmid of the closest point,
  # which is used for joining to nodes
  samplePointsData['closest_node_id'] = np.where(
      samplePointsData.n1_distance <= samplePointsData.n2_distance,
      samplePointsData.n1, samplePointsData.n2)

  # join the two tables based on node id (join on sample points and nodes)
  samplePointsData['closest_node_id'] = samplePointsData[
      'closest_node_id'].astype(int)

  # first, join POIs results from nodes to sample points
  samplePointsData = samplePointsData.join(gdf_nodes_poi_dist,
                                           on='closest_node_id',
                                           how='left',
                                           rsuffix='_nodes1')

This appears to be saying, given the closest node, assume that because its the closest to this sample point then the distance from this node to a destination will be the best estimate for distance from this sample point to a destination.  Then, it appears that the distance from the sample point to that closest node is also disregarded.
I think this is wrong in a few ways, and it would be great to get others thoughts.
Each sample point has been generated on a network edge, and as such each only has two possible nearest nodes - located at the terminating ends of the line segment.
e.g.  this point is associated with nodes N1 (10 units away), and N2 (37 units away)
Line start (node 1, N1) o----------point-------------------------------------o Line end (node 2, N2)
The reason why it is invalid to just assume that the nearest node provides an adequate estimate of shortest distance to a destination is because unless access through both of these nodes is evaluated, respectively summing on the distance of the two nodes to the sample point, then the true answer of which node is closer cannot be known.  SpatialVision do neither of these things; however, that is the reason they were provided with the associated nodes and the distances from the sample points, so they could calculate the two potential full minimum distances, and then take the smaller value of the two.
For example, SpatialVision's method appears to me to assign the distance from N1 to the closest supermarket directly as an estimate to the sample point.  But what if the closest supermarket to both N1 and N2 is 10 units to the right of N2?  That would mean the distance for N1 is 57 units, and the distance for N2 is 10 units.  Taking the minimum distance from the two points alone is not sufficient --- you have to consider the distance first from the point to the nodes
Scenario A (N1): 10 + 10 +37 +10 = 67 units (including backtracking, which doesn't make sense - but its how this method works)
Scenario B (N2): 37+ 10 = 47 units
So, Scenario B shows that N2 is closer to the supermarket; you take the minimum distance of the two fully accounted for distances as the sample point's distance to closest estimate.
There is an exception to this logic --- regardless of whether N1 and N2 are closer to the supermarket, if its found that the sample point and the supermarket are both located on the same network edge, the supermarket may be closer to the sample point than either N1 or N2.  It may not be the case, but it probably is and to be thorough, it could be considered.
So maybe that is
Scenario C (if sample point shares line segment with destination): Euclidean distance
Technically, a line could curve, and Euclidean distance might be wrong, but for a single edge segment we would expect any such error to be very small.
Example
Here's an applied example of how SpatialVision's method could be very problematic.
In the image below, there is a road segment 1120  metres in length, and contains 38 sample points; there are also two supermarkets nearby.  The two terminal nodes are bright yellow.

The left-most node is about 380 metres from a supermarket, the right-most node is about 1100 metres away.
I think without doing the math, that to figure out which sample point on the line should go to which supermarket ... you have to do the math, adding on the full distances to the two options for each sample point, and then taking the minimum as the final answer.
In contrast SpatialVision assign half the points to node 1 ("380 metres away") and half to node 2 (1100 metres away).  This actually defeats the purpose of our sampling resolution which is to have continuous estimates, but is also inaccurate:  the two points in the middle of the line  --- one should have been about 880 metres away (a false positive for accessibility using SV method), and the other is similar, say 910 metres (coincidentally for SV, a true negative - but they actually matched to totally different supermarkets, as the closest is accessed via N1 in both instances, despite the second node being nominally closer to N2).
What I think should be done is not much more computationally complex (as for now, I will overlook scenario C).

replace lines 209-215 with expansion of the samplePointsData dataframe to long form, with each point having two rows (for N1, and N2), and the node IDs as recast as integers
join with the gdf_nodes_poi_dist data frame (like on line 218)
sum each node amenity distance to the sample point distance variable
for each sample point, take the minimum full amenity distance of the two options

that's it! It shouldn't take much longer and would be much more accurate (it's actually my understanding of what we asked SpatialVision to do; hence why we provided them with the variables in order to calculate this, rather than just pre-associate with closest node).
There could be extension of the above to account for the occasional Scenario C situation --- that distance could perhaps be appended where applicable to the long frame result dataframe before summarising to find the minimum full distance for each amenity.
I'll see if I can implement a proof of concept version of this tomorrow - but also keen to hear your thoughts as a sanity check in case I am misreading what SpatialVision have done.  I am pretty sure I'm not though...

I've now processed Bangkok using the existing SpatialVision script so I can provide a better visual example of the issue I described above.

We see that sample points have not been evaluated with a continuous distance (which is do-able given the available data), and by just using the distance of the nearest node the use of these sample points is made both redundant and invalid.  You can see that despite access being evaluated as "411.48001..." metres, for some of the points on this line the true distance is more than a kilometre; in facty, most of them are not within 500 metres.
There are two other issues I think we should fix related to this

"411.48001" is an unecessary precision given our margin of error is larger than a metre -- we should store these as integers and save space and processing power
It is a very bad idea to store distance estimates as -999, as a logical thing for us to do is evaluate for access < 500 metres.  It would make sense to have those sample points with out access to have a result of nan.  This would mean we would have to change our use of dropping nans - but as Shirley has demonstrated, this is problematic too.

As mentioned I'll have a go to see if I can make a modification to ensure the full distances are recorded; I'll also make sure distance estimates are only integers.  I won't touch the -999 thing for now, as that is really a separate issue (i'll log it as such, along with another less urgent data management issue).

I've now re-calculated Bangkok using the sketch revised method (pushed up to 'full-distance-fix' branch of repository); only quickly looked at this so far, but want to share how this looks visualised using the full distance layer:

In this image points with access within 500m are coloured blue through green with larger circles; those without full distance access (either null, or full distance > 500m) are small yellow dots.  The fresh food / market destinations are the large red stars to aid visibility; I will refer to these as 'supermarkets' for ease (as this is what they are called in the code).
Regarding the points on the line we looked at above, we see only a few at the far left have access to a supermarket within 500m once the full distances including distance to nodes are accounted for; using the SV method, half of them did, wrongly.
We see another issue on the eastern side which is not resolved however; this is a bit like the 'scenario C' which I mentioned --- where a destination is (or should be) associated with a point on an edge and so be evaluated using Euclidean distance for like points.  What has actually happened in this instance is interesting --- the destination (a bakery, it seems) is actually recorded off a busy road on the network --- then, when snapped by Pandanas for evaluating distance, the nearest node is someway down the road - and depending how you count it, this "nearest node" is either 80 or 100 metres away, and located on the other side of a canal, and (according to OSM) not connected to the actual road segment which the destination should have been snapped to:

So we see, some of these points were allocated access within 13 m or more:

Which despite being a problem is still a small improvement over Spatial Vision method where all were allocated 0m access:

The resolution for these kinds of issues isn't immediately apparent (and is another issue) but some options could be to consider could be somethink like:

pre determine closest point on closest edge segment for each point, and the distance to this (e.g. in above case, about 30m)
pre-determine the two terminal nodes on this edge segment and consider them 'surrogate destinations'; we record the respective distances from the snapped point, summed to distance to original point (which may not have been located on line)
run pandanas access calculation using surrogate nodes as destinations, but sum the pre-recorded surplus distances as an offset to any access distances recorded (if this is possible??)
then proceed as per the revised 'full-distance-fix' method above

Alternately, we could ignore and note as a limitation.
As far as the 'full distance' issue, I don't necessarily think this will make a big difference for marginal city averages, however I think , but I think it is a source of error in the spatial distribution (eg. hex averages) which we could do without
Here is a summary of spatial distribution of difference (0 difference hidden) for Bangkok - using SV access to fresh food minus revised access to fresh food:

You can see from the legend, that most points estimates are not impacted; however, just under 0.83% (n = 5,949) are evaluated after considering the 'full distances' as having access when they don't, and 0.36% (2541) are recorded as not having access, when they do.  As discussed, this doesn't yet account for proximally co-located points (Scenario 3), which are a seperate source of error.
Regarding Daily Living, 95.9% of sample points have an unchanged score; however, 3% are positively biased (higher access scores than they should be, in most cases by 1 point, but up to 3,  (about half as many again are negatively biased).  Here is spatial distribution of difference, omitting no difference:

I believe the sketch implementation works, but as I have changed field names it may not just drop into existing workflow.  I will wait to hear what others (you all) think before working any further on this (in case you think there are better approaches).
Personally, I think we should adopt this; it would also be nice if we could resolve the other access error described above.  I don't anticipate large differences to city-scale summaries, but the value of this work is in the spatial distribution mapping of relative inequities, and as such I think we should try to eliminate those sources of bias which we can.  This is particularly important, given our knowledge of data limitations.

Carl, thanks for these detailed explanations! I recognized this problem as I was testing out the Pandana method, but did not consider it further because we were also looking at the sausage buffer approach as an alternative for accessibility back then.  I understand why SV did that as that's a convenient way to accommodate the use of the Pandana package.  As we are heavily relying on the Pandana approach, I agree as you said it is important that we need to pay more attention to its limitations. But I also tend to perceive this issue as the level of precision rather than a source of error.
Could you give me the link to your 'full-distance-fix' branch? I am not sure where I can find it... this reads a good solution to me, as far as it improves the precision of the accessibility measure, why not?
As I am going through the code commenting now, I will also put more thoughts into the other issues as you raised here.

Hi Shirley, can you access the 'full-distance-fix' branch here?
https://github.com/gboeing/global-indicators/tree/full-distance-fix

Hi, Carl, I reviewed and tested your full distance approach, it works well. I am thinking the new codes could be simplified in some way, such as changing the sp field names in the config,  writing the new workflow into a function like get_sp_poi_min_full_dist, just to be consistent with our practice.  But I know these are sketch versions, and I assume you are still working on this.
Anyway, I agree that's a good idea, I will leave it up @gboeing for any further comments and if we should adopt this approach for accessibility, i.e. distance to nearest node vs. full distance (sp-nodes-nearest pois).
On another subject, I see another change in your branch about the weighted mean of pop and intersection density, I am a bit lost from here, could you explain your work on this part?
Thank you!

Hi Shirley,
Thanks for having a look at the code.
Regarding your question, as I was working on amending the sample point attribution of full distances, rather than just those of the nearest node (which may be hundreds of metres away), I realised that this is also a problem with the intersection and population density measures that were being attributed.
The simplest improvement that occurred to me was to take an average of the two closest nodes measures, giving commensurate weight to the nodes' respective distances.  For example, if one node is 30m away, and the other is 70m away, the first node is a better candidate for the experience at the location of the sample point (1-30/(30+70)= .7) but under the assumption that there is some continuum of experience and that some degree of interpolation between the two points is appropriate, we allow the second node to contribute also, to a lesser degree (1-70/(30+70)= .3) .  It solves the problem of a 1km stretch of road with sample points either being snapped to a value at one end or the other end of the road, regardless of whether they are in the middle or truly near the terminal nodes.  In truth, we don't know what the experience is in the middle of the road as it hasn't been measured; however, our best guess is some average of the two nearest nodes.  Then, as you head from the middle stretch of the road closer to another node, that node's contribution to the built environment measure becomes dominant.  It assumes there is some gradient of density, rather than a step function at the road's mid point (the implication of spatial vision method).
Hope that makes sense without too many typos :) (written on my phone)
I'll be happy to implement my sketch as a function if we agree it's a good approach; to save wasted effort (and because I have other projects to work on) I decided to wait for feedback before doing this.
Thanks again for checking it out; I'm open to other suggestions. I think we can and should improve on the method implemented by Spatial Vision.

I have just pushed up a revision to the master branch, in a new branch 'full_dist_function' which I believe addresses the above issue (incorrect naive association of sample point with nearest node estimate) for distances and densities.  In addition the new fix does not remove the nan distance estimates since, as far as I can tell these only arise in cases of legitimate interpretation as "no access".  I will be interested to see what others think.
Here is a snapshot

In the above image, you can see both the previous SpatialVision code base method (calculated using the most recent master code), and the revised version colourised using the same scale (left of image).  The original sample points are smaller, the revised sample points are larger but on a lower layer so where you see a disjunct coloured ring around a sample point this represents a change in estimate.
In general, you see that where there are sample points located on longer stretches of road, these tend to have their access to destinations considerably over estimated.  As noted in above posts there are cases where the 'snap to closest' method can also under-estimate access (under incorrect assumption that closest intersection is the best route to a destination).  However, in general the original SpatialVision method is biased upwards - as reflected in median walkability of 0.39, compared with 0.34 for revised version.
In the revised version, instead of assuming the closest intersection is the best route, the distance from sample point to each of its two nearest intersections is added to their respective distances from each destination and then the shortest of the two 'full distances' is chosen as its access estimate, which is then evaluated against a threshold to create a binary indicator of access.  Where the distance from an intersection to a destination already exceeds this threshold it is interpreted as not having access by that route, regardless of distance from sample point to that intersection
The SpatialVision method dropped some records where they recorded NaN (null) for access to destinations.  However, in my checks, these all appear to be legitimate cases of 'no access recorded' and should not be dropped, but instead retained to reflect the information that they implicitly represent: there is no access within the required distance.  The NaNs did not seem to be systematically recorded - for example in the above snapshot, they appeared to be associated with intersections with no access to convenience stores (not necessarily all that didn't have access, but some).  This is concerning, and in the absence of seeing a valid reason for exclusion, I think these make sense to be retained as reflecting no access.  If others have more insight into what is going on here it would be great to hear your thoughts.
The snapping to nearest intersection did not just impact distances, but also densities.  Ideally, the densities would be measured from the sample points - not the nodes.  However, the code has been heavily set up to measure these from the nodes and then snap the nearest intersection node's evaluated dwelling and intersection densities to each sample point.  The best way I could think to quickly and easily produce a more rigorous estimate was to use the densities of the two nearest nodes, and take an average giving proportional weight based on proximity (e.g. if one node is 300 metres away and the other 100 metres away, then the node 100 metres away contributes more (75%) than the node 300 metres away (25%))..  It is not a perfect solution, but it is a simple model reflecting the truth that closer things are more similar than further things, but even those that are slightly further away still contain useful information - and if two things are practically equidistant, they should both be counted, not one arbitrarily chosen.
I expect in practice this won't have a large impact, but I also think it is an important improvement to the model.  Thanks for your patience while I implemented this.  I'm keen to hear your thoughts.

I've just realised, differences in the full distance fix branch include changed variable names; these were done in my draft script, and will cause issues if not accounted for in the aggregation script --- when implementing the function I overlooked the naming convention.  For now, I will leave this, as there were a few important changes and its important to make sure all are on board before accepting.  Then if its thought good in principle I could revise the naming conventions before pulling into master perhaps.

I have pushed up a version of the full_dist_function branch based on the new (current as of 20 April 2020) master.  However, there may be issues with the now retained estimates which were previously dropped -- I am not sure that there is anything amiss with the distance estimates for these, however, it appears that these nodes are associated with missing estimates for intersection and pop density saved to the csv file (osmids are associated with blank values) which I suspect is resulting in erroneous values for these entries.  The commit contained a confusing note relating to something not being right, however that didn't make sense really --- in short, I think more should be investigated as to why these nodes are being dropped - I'm not yet sure why, but I think it might be important as there should be no reason that these nodes should not have valid values for all fields as far as i can tell (the examples i have checked appear connected).

All of the dropped NA points in Vic are located on edges with 'oneway=True' ; that is, routing is being evaluated using a directed not undirected graph.
The failure occurs in the current implementation of calc_sp_pop_intect_density_single(), where the following code
   # create subgraph of neighbors centered at a node within a given radius.     subgraph_proj = nx.ego_graph(G_proj,                                  orig_node,                                  radius=length,                                  distance='length') subgraph_gdf = ox.graph_to_gdfs(subgraph_proj,                                 nodes=False,                                 edges=True,                                 fill_edge_geometry=True)
which will return only the source node, and no path if that node is at the end of a one way road segment which has no logical connection to rest of network.  That is, subgraph_gdf is empty in such circumstances.
The code accounts for this scenario with the check
if len(subgraph_gdf) > 0:
and in the case of no subgraph existing, return (np.nan, np.nan).
This causes issues because it is not true that the network is directed and there should be neighbourhood measures associated with these nodes, and their absence is problematic for evaluation of street connectivity and population density.  It could be expected to introduce both within- and between city bias, relating how and where oneway roads are implemented; it is assumed that in most circumstances a oneway road should be no impediment for a pedestrian.
I believe there is a simple fix, if we convert G_proj to undirected, after loading the graph in function read_proj_graphml()
G = ox.get_undirected(G)
I'm testing this now (waiting for it to process), and my hypothesis is, with this in place we won't get any nodes lacking intersection and population density estimates; in turn, I believe we won't have any results with na values, and as such no results that would have been dropped due to these.
I'll push my code up with these amendments for now; however, I'll update on here once I've verified that this simple fix will address the issue.  Fingers crossed!

I've just verified for Vic that the new fix appears to resolve the issues observed (discontinuities in expected values, arising from some key nodes being omitted where they are located on edges tagged as oneway --- ie. a directed graph was being used instead of an undirected graph for evaluating the local neighbourhood).
The easiest way to view the difference for Vic is to compare two maps --- and turn on and off the old layer; but i'll see how it works as two separate images here.
First, the old 'dropped na' points (sample points who were matched to a closest node which for whatever reason returned no intersection or population density estimate):

The previous version of full distance fix, but still using the directed graph (not the discontinuities of walkability in area where the dropped nas are --- those are on one way streets!!):

The revised version, calculated using an undirected graph --- the shading of walkability variation is much smoother, as expected.

There are additional changes which were not anticipated in this version though - see the shift in hue on the periphery?  It seems using the undirected graph, for whatever reason in general the estimates of walkability are in general lower.  Perhaps this is explained as since the outlying very negative values are no longer present in the inner city, this has resulted in some shifts in the overall distribution when calculating z-scores?  I am surprised that that much of an effect would be observable --- however, the colour scaling of the two maps above are both using the same breaks:

In any case - the new version seems more plausible; and where you compare the csv file , if you tally up the number of results in the new dataset compared with the old, there were 95 more of the nodes which were associated with values which were NA when using the undirected graph.  These will be the nodes that some of the 'dropped NA' sample points in the original spatial vision implementation would have been snapped to.  In the revised implementation (mk1), results are not just based on closest node (for distance, it depends on optimal route via either proximal nodes; for densities it is a weighted average favouring proximity between the two nearest nodes).  In revision mk2, we corrected for the undirected graph  - and the resulting map makes sense.
I am not sure why the other node values in the CSV are null exactly -- however, in my quick checks of the remaining entries without results, I could not locate these nodes' osmid values in the dataset of nodes within the 1600m buffer of the study region (in the geopackage); those nodes I checked which had values were in that dataset.  So I suspect the 7179 nodes (51%) with no density records in the final current CSV for Vic relate to the GraphML file being for the 10,000m buffered study region.
Given we only need to process those node OSM ids within the buffered region, there are likely some large efficiency gains to be had by restricting the local neighbourhood analysis of nodes to those node ids within 1600m of the study region (ie. load up only the rows from gdf_nodes_simple matching node ids from the geopackage, and use those for the iteration list).
That's an optimisation for another time though i reckon, once we agree on the above changes!
So this change has been pushed up on the 'full_dist_function' branch.

I believe there is a simple fix, if we convert G_proj to undirected

Given that our theoretical "agent" (i.e., traveler) in this study is an active traveler using nonmotorized transport, it seems reasonable that we make all the graphs undirected. The directionality constraints only really make sense if you're modeling driving trips that must obey directed travel. So I think your proposal makes sense!
My question is: why do we have directed graph models? If we are creating these models with OSMnx, are we using the parameter network_type="walk"... if so then the models should be undirected in the first place.

Thanks Geoff,
We're on the same page re the theoretical freedom of walking!  Unfortunately, there are some snags in how I have implemented this, it turns out.
To answer your questions in reverse, the code is in the 'global' branch of the Bangkok bitbucket repository which I believe I have previously granted you access to; in particular, https://bitbucket.org/carlhiggs/ind_bangkok/src/global/process/03_create_network_resources.py
There are some complications, and I now see where the issue has arisen:
As you know, the OSMnx 'walk' and 'bike' network type definitionsare a bit mutually exclusive -- the walk network excludes cycleways, and the bike network excludes footways:

    # walking: filter out cycle ways, motor ways, private ways, and anything
    # specifying foot=no. allow service roads, permitting things like parking
    # lot lanes, alleys, etc that you *can* walk on even if they're not exactly
    # pleasant walks. some cycleways may allow pedestrians, but this filter ignores
    # such cycleways.
    filters['walk'] = ('["area"!~"yes"]["highway"!~"cycleway|motor|proposed|construction|abandoned|platform|raceway"]'
                       '["foot"!~"no"]["service"!~"private"]{}').format(settings.default_access)

    # biking: filter out foot ways, motor ways, private ways, and anything
    # specifying biking=no
    filters['bike'] = ('["area"!~"yes"]["highway"!~"footway|steps|corridor|elevator|escalator|motor|proposed|'
                       'construction|abandoned|platform|raceway"]'
                       '["bicycle"!~"no"]["service"!~"private"]{}').format(settings.default_access)

While it is noted that the some cycleways may allow pedestrians, we found based on previous investigation of the use of OSM for active transport analysis in Melbourne, that the 'walk' network in OSM excluded some important paths where people could otherwise walk because these had only been tagged in OSM as for cycling (e.g. paths crossing a river, tagged in OSM for cycling but which are in reality dual use).   On reflection, it seemed to us that in our urban experience it is almost always the case that pedestrians can go where bicycles go (if not literally on the same path, on one parallel), although the reverse doesn't hold (ie. cyclists are not allowed on all paths).  While there may be instances where this doesn't hold, we felt the benefits of capturing informal paths suitable for active transport using OSM which were not present in the official road network datasets more accurately were achieved by including, rather than excluding cycleways.
So, after experimenting and realising that creating a union of the 'walk' and 'cycle' networks was ineffective, I realised I could achieve something equivalent for our purposes using a custom definition:
# define pedestrian network custom filter (based on OSMnx 'walk' network type, without the cycling exclusion)
pedestrian = (
             '["area"!~"yes"]'
             '["highway"!~"motor|proposed|construction|abandoned|platform|raceway"]'
             '["foot"!~"no"]'
             '["service"!~"private"]'
             '["access"!~"private"]'
             )

We implemented the use of the networks generated using this definition into our existing analysis workflow, which for our Australian project involved the use of the 'edges' and 'nodes' being exported as shape files, and following projection into the project CRS using these in conjunction with ArcGIS network analyst for running undirected network analyses.
I see now that the above approach overlooked that the network type in OSMnx is also used to define whether a network is bi-directional or not in the graph_from_polygon function which I used:
G_buffered = create_graph(response_jsons, name=name, retain_all=True,
bidirectional=network_type in settings.bidirectional_network_types)
Using a custom filter, this meant that network type was not listed in the pre-defined list of bidirectional network types.  Therefore the graphml was saved as a directed graph, unintentionally.  Since I hadn't used the graphml for network analysis formally on a project until now (ie. beyond basic experiments), I have only just realised the issue that this has caused.
So we have options, which could include variations on the below:

Use the existing custom pedestrian networks (including cycling, under assumption that such paths in most instances are walkable, and may be important for connectivity), and force the existing networks to be undirected (this is what the code recently pushed does).  I lean to this option myself for the moment, if we think the forcing the network to be undirected is a viable fix, for two reasons 1) I think the 'highway=cycleway' tag is used on OSM in ways where this gives practical hints as to an area that could be walkable, but may not otherwise be tagged as such (based on my experience); 2) it means we don't have to re-do work

Re-do the networks using the default 'walk' network type.  This would mean we could take the opportunity to reduce the buffer size which would reduce file sizes and redundant memory usage.

However, I think the omission of highway=cycleway is problematic. While inclusion of this tag may seem counter-intuitive, it isn't intended to be mutually exclusive to walking; rather, 'Tagging a way with highway=cycleway implies that the route is designated for bicycles.'.  which prompts the question --- if there is an edge which is tagged only with highway=cycleway, is it more realistic to include this or exclude this when modelling a walking environment?  And if the exclusion leads to real walkable paths being excluded will our bias against walkability be greater than the bias for walkability occurring had we included them?
One example which I think is useful is this path, which I expect would not be included in the current 'walk' network type default implementation due to its tagging as 'highway=cycleway', even though it is (now, I don't think it used to be) tagged with 'foot=designated':

Another alternative could be to create an alternative 'active transport' network type in OSMnx which removes the cycleway restriction, and include this in the list of bi-directional network types.  Then it would be up to researcher discretion as to whether 'walk' or 'active transport' is more appropriate for their pedestrian modelling purposes.
But - in short, please accept my apologies for the creation of these networks as directional; this was not my intention.  I'm glad we found the issue sooner than later.
How do you think we should resolve this?

No worries! Let's think this through...
I think your first option makes the most sense. By the way, a related option would be to specify a custom_filter to include all traditionally walkable routes + all cycleways and then specify network_type="walk" to make the entire graph bidirectional. This is all feasible with OSMnx natively as-is. See here for example: https://github.com/gboeing/osmnx/issues/169
I also like the idea of defining a new 'active' network type in OSMnx. One of the hold-ups in doing this in the past was uncertainty around directionality. That is, if you're walking you can obviously go in either direction. But if you're cycling it's less clear. Making the whole thing bidirectional might misrepresent cycling routing, but making the whole thing unidirectional would certainly misrepresent walking routing.

Thanks Geoff -- good points, it makes sense you've thought through these issues before.  That totally makes sense now that network type can still be used to define directionality with a custom filter as an override --- I should have posted that question back in 2018 myself!  I'm glad that for now it seems that we can still use the already processed resources without having to re-do these, but I'll amend the OSMnx network construction code for future work to also specify network_type="walk"  - excellent :)

Hi all - as follow up, I believe the 'full_dist_function' branch is ready to be pulled (as of 20 April), however as this is an upstream script to the aggregation scripts I think it would be useful if others assisted to help confirm that the final data structure don't cause unintended conflicts.  I will attempt to look at this today, but I would appreciate if others could consider this too.   (Please note that the 'full_dist_fix' is an old branch, based on the SpatialVision codebase I believe prior to Shirley's refactoring, which I will attempt to remove).

@carlhiggs are you going to open a PR for everyone to review the diff? Or what other assistance would you need?

I've just made the pull request for full_dist_function to make it easier for others to review; it looks like there aren't overt conflicts which is great.  But I anticipate we may have to tweak things at the aggregation end.

Thanks @carlhiggs. For reference his PR is #48.

Thanks @gboeing ; this is the first project where I've worked with others actively contributing to code in this way (I've mostly been sole contributor to repository, so reviewed and merged branches directly), and so I am learning the etiquette of pull requests etc and "@"ing.  Thanks for bearing with me.

no worries!

I believe this issue has been addressed by the full distance function branches which have since been merged into master; so, marking as closed.  Thanks!


fix: js-yaml issue after upgrade

Please check #11 and #12

:tada: This PR is included in version 1.1.5 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


Unable to select value picker value

Description
Using detox to select a value from a picker appears to be broken on IOS.  Using a testID on a Picker or using UIPickerView have different results as seen below. This could be a result of any number of things going wrong in the dependency chain, but I figured the detox community may know the answer.  I've added a spec to the e2e tests to isolate the issue.  You can find the branch here.
Steps to Reproduce

Clone repo
Switch to branch failing-e2e-picker-test
Follow instructions to build/run tests

Detox, Node, Device, Xcode and macOS Versions

Detox: 7.4.3
React Native: 0.51 (in detox repo)
Node: 8.11.1
Device: iPhone 8 Plus - 11.4
Xcode: 9.4.1
macOS: 10.13.5

Device and verbose Detox logs
✗ :ios: Picker picker should trigger change handler correctly using testID
 ● :ios: Picker › picker should trigger change handler correctly using testID

    Error: Cannot find UI element.
    Exception with Action: {
      "Action Name" : "Set picker column 0 to value 'Bar'",
      "Element Matcher" : "(((respondsToSelector(accessibilityIdentifier) && accessibilityID('FooPicker')) && !(kindOfClass('RCTScrollView'))) || (kindOfClass('UIScrollView') && ((kindOfClass('UIView') || respondsToSelector(accessibilityContainer)) && ancestorThatMatches(((respondsToSelector(accessibilityIdentifier) && accessibilityID('FooPicker')) && kindOfClass('RCTScrollView'))))))",
      "Recovery Suggestion" : "Check if the element exists in the UI hierarchy printed below. If it exists, adjust the matcher so that it accurately matches element."
    }

    Error Trace: [
      {
        "Description" : "Interaction cannot continue because the desired element was not found.",
        "Error Domain" : "com.google.earlgrey.ElementInteractionErrorDomain",
        "Error Code" : "0",
        "File Name" : "GREYElementInteraction.m",
        "Function Name" : "-[GREYElementInteraction matchedElementsWithTimeout:error:]",
        "Line" : "124"
      }
    ]

      71 |     // when this test run fails, we want a stack trace from up here where the
      72 |     // $callee is still available, and not inside the catch block where it isn't
    > 73 |     const potentialError = new Error()
      74 |
      75 |     try {
      76 |       await this.sendAction(new actions.Invoke(invocation));

      at Client.execute (../../src/client/Client.js:73:28)
      at InvocationManager.execute (../../src/invoke.js:11:33)
      at ActionInteraction.execute (../../src/ios/expect.js:178:29)
      at Element.setColumnToValue (../../src/ios/expect.js:306:86)
      at Object.it (20.picker.test.js:10:39)

send: {"type":"invoke","params":{"target":{"type":"Invocation","value":{"target":{"type":"EarlGrey","value":"instance"},"method":"detox_selectElementWithMatcher:","args":[{"type":"Invocation","value":{"target":{"type":"Class","value":"GREYMatchers"},"method":"matcherForAccessibilityID:","args":[{"type":"NSString","value":"FooPicker"}]}}]}},"method":"performAction:","args":[{"type":"Invocation","value":{"target":{"type":"Class","value":"GREYActions"},"method":"actionForSetPickerColumn:toValue:","args":[{"type":"NSInteger","value":0},{"type":"NSString","value":"Bar"}]}}]},"messageId":3}
 onMessage: {"type":"testFailed","messageId":3,"params":{"details":"Cannot find UI element.\nException with Action: {\n  \"Action Name\" : \"Set picker column 0 to value 'Bar'\",\n  \"Element Matcher\" : \"(((respondsToSelector(accessibilityIdentifier) && accessibilityID('FooPicker')) && !(kindOfClass('RCTScrollView'))) || (kindOfClass('UIScrollView') && ((kindOfClass('UIView') || respondsToSelector(accessibilityContainer)) && ancestorThatMatches(((respondsToSelector(accessibilityIdentifier) && accessibilityID('FooPicker')) && kindOfClass('RCTScrollView'))))))\",\n  \"Recovery Suggestion\" : \"Check if the element exists in the UI hierarchy printed below. If it exists, adjust the matcher so that it accurately matches element.\"\n}\n\nError Trace: [\n  {\n    \"Description\" : \"Interaction cannot continue because the desired element was not found.\",\n    \"Error Domain\" : \"com.google.earlgrey.ElementInteractionErrorDomain\",\n    \"Error Code\" : \"0\",\n    \"File Name\" : \"GREYElementInteraction.m\",\n    \"Function Name\" : \"-[GREYElementInteraction matchedElementsWithTimeout:error:]\",\n    \"Line\" : \"124\"\n  }\n]"}}

✗ :ios: Picker picker should trigger change handler correctly using type
 ● :ios: Picker › picker should trigger change handler correctly using type

    Error: An action failed. Please refer to the error trace below.
    Exception with Action: {
      "Action Name" : "Set picker column 0 to value 'Bar'",
      "Element Matcher" : "((kindOfClass('UIPickerView') && !(kindOfClass('RCTScrollView'))) || (kindOfClass('UIScrollView') && ((kindOfClass('UIView') || respondsToSelector(accessibilityContainer)) && ancestorThatMatches((kindOfClass('UIPickerView') && kindOfClass('RCTScrollView'))))))"
    }

    Error Trace: [
      {
        "Description" : "UIPickerView does not contain desired value!",
        "Error Domain" : "com.google.earlgrey.ElementInteractionErrorDomain",
        "Error Code" : "2",
        "File Name" : "GREYPickerAction.m",
        "Function Name" : "-[GREYPickerAction perform:error:]",
        "Line" : "120"
      }
    ]

      71 |     // when this test run fails, we want a stack trace from up here where the
      72 |     // $callee is still available, and not inside the catch block where it isn't
    > 73 |     const potentialError = new Error()
      74 |
      75 |     try {
      76 |       await this.sendAction(new actions.Invoke(invocation));

      at Client.execute (../../src/client/Client.js:73:28)
      at InvocationManager.execute (../../src/invoke.js:11:33)
      at ActionInteraction.execute (../../src/ios/expect.js:178:29)
      at Element.setColumnToValue (../../src/ios/expect.js:306:86)
      at Object.it (20.picker.test.js:18:44)

send: {"type":"invoke","params":{"target":{"type":"Invocation","value":{"target":{"type":"EarlGrey","value":"instance"},"method":"detox_selectElementWithMatcher:","args":[{"type":"Invocation","value":{"target":{"type":"Class","value":"GREYMatchers"},"method":"detoxMatcherForClass:","args":[{"type":"NSString","value":"UIPickerView"}]}}]}},"method":"performAction:","args":[{"type":"Invocation","value":{"target":{"type":"Class","value":"GREYActions"},"method":"actionForSetPickerColumn:toValue:","args":[{"type":"NSInteger","value":0},{"type":"NSString","value":"Bar"}]}}]},"messageId":6}
 onMessage: {"type":"testFailed","messageId":6,"params":{"details":"An action failed. Please refer to the error trace below.\nException with Action: {\n  \"Action Name\" : \"Set picker column 0 to value 'Bar'\",\n  \"Element Matcher\" : \"((kindOfClass('UIPickerView') && !(kindOfClass('RCTScrollView'))) || (kindOfClass('UIScrollView') && ((kindOfClass('UIView') || respondsToSelector(accessibilityContainer)) && ancestorThatMatches((kindOfClass('UIPickerView') && kindOfClass('RCTScrollView'))))))\"\n}\n\nError Trace: [\n  {\n    \"Description\" : \"UIPickerView does not contain desired value!\",\n    \"Error Domain\" : \"com.google.earlgrey.ElementInteractionErrorDomain\",\n    \"Error Code\" : \"2\",\n    \"File Name\" : \"GREYPickerAction.m\",\n    \"Function Name\" : \"-[GREYPickerAction perform:error:]\",\n    \"Line\" : \"120\"\n  }\n]"}}

Did you try to inspect the view hierarchy and see what's going on ?

I will have a look at the view hierarchy and report back.

TBH, I am not sure what would be helpful for debugging here.
Both have a hierarchy of UIPickerView with UIPickerColumnView.  I would expect executing await element(by.type('UIPickerView')).setColumnToValue(0, 'Bar'); would work for Picker as it does for choosing a time in a date picker as follows (which works) expect targeting a different column.
  await element(by.type('UIPickerView')).setColumnToValue(2, minute)
  await element(by.type('UIPickerView')).setColumnToValue(1, hour)
  await element(by.type('UIPickerView')).setColumnToValue(3, meridiem)

It may be possible that react native doesn't pass testID to native view hierarchy on one of them, but does on the other.
It may also be possible that the testID is not being added at the same hierarchy path (it me set on a wrong view, like a wrapper view).
Anyway, different UI element implementation may have different behavior. It's best to understand the native layout hierarchy with the inspector

The test attempts 2 ways to find the element.  by.id causes the test to fail because if does not find the element.  This makes sense if the testID is not passed to a native element.  by.type fails because it is unable to select the value in the picker.  Given that it finds the element and the values are present I'd figure there is something else going on.

Can you check the accessibility properties of the view you're trying to match? Let's see the value in the picker

These are not from the test, but are built using the same react-native Picker and have the same problem outlined in the test.  Using setColumnToValue(0, '2') or setColumnToValue(0, 'Week') will produce the same error "UIPickerView does not contain desired value!".  There does not appear to be anything out of the ordinary to me.


Improve performance of variable resolver

Switch to a hash table when the number of variables grows beyond a threshold.
Speeds up the test case from the linked issue by about 70%.
Fixes: https://github.com/quickjs-ng/quickjs/issues/456

I'm going to tweak it a little more before I land it, stay tuned.

Nice! Random question: we seem to have a few htables around now, do you see an opportunity to DRY some out? Not in this PR of course.

This would be great. Maybe even pulling in something external? https://github.com/JacksonAllan/Verstable :D

Dunno. They're all just different enough from each other that it's not a slam dunk DRY job.
I did notice that, compared to perl hash, shape_hash and get_index_hash have like 5x the number of hash collisions on the (limited) inputs I tested it with.
Swapping the hash function didn't make a measurable difference in my quick testing though, likely because the shape and IC hash tables just don't get that big.


Migrate trace list response processing

Sure I'll add to the description. Please note this is a task of the epic https://github.com/grafana/opensearch-datasource/issues/223


apollo-link dependency causing multiple versions of graphql under node_modules

Since this PR:
https://github.com/apollographql/graphql-tools/pull/409
My server is using graphql 0.11.7, and latest graphql-tools, and now it comes with a dependency for apollo-link, causing my node_modules have multiple copies for GraphQL:
./node_modules/apollo-server-core/node_modules/graphql
./node_modules/apollo-link/node_modules/graphql
./node_modules/graphql

Causing the server to throw:
Schema must be an instance of GraphQLSchema. Also ensure that there are not multiple versions of GraphQL installed in your node_modules directory.

I believe this would require changing apollo-link to list graphql as a peer dependency (may be in progress here: https://github.com/apollographql/apollo-link/issues/78) and then updating to use the newer apollo-link. This is a similar problem to what react encountered early on, and now most react libraries list react as a peer dependency.

Ping @jbaxleyiii @peggyrayzis

Also, as a note, the current declaration of apollo-link as a dev dependency cause the following typescript error since it is not installed by default. Changing to a peer dependency would solve this problem as well.
node_modules/graphql-tools/dist/stitching/introspectSchema.d.ts(2,28): error TS2307: Cannot find module 'apollo-link'.
node_modules/graphql-tools/dist/stitching/makeRemoteExecutableSchema.d.ts(1,28): error TS2307: Cannot find module 'apollo-link'.

Any updates ?

Should be fixed in 2.7.0.

Seems broken in 2.7.1

I agree that it's broken in 2.7.1. In previous version there was no problem with it:
node_modules/graphql-tools/dist/stitching/introspectSchema.d.ts(2,28): error TS2307: Cannot find module 'apollo-link'.
node_modules/graphql-tools/dist/stitching/makeRemoteExecutableSchema.d.ts(1,28): error TS2307: Cannot find module 'apollo-link'.

We had to remove apollo-link from peer dependencies, because nested peer dependencies broke on some environments. graphql-tools only needs apollo-link types, so for typescript you can just install it as devDependency. I'm sorry for the inconvenience.

I was referring to original issue when I said it was fixed. Sorry, I didn't read further messages :)

As the original issue is not fixed via the change of dependency, I have created a new issue (#472) for this particular problem.

#472

The dependency is gone, but there is indeed a new issue with TypeScript. Thanks for opening a new issue!


Online Callback Check

Display an undismissable pop-up if device is offline. Currently app gets stuck

Fixed - Added Connectivity Callback


[2] Upstream: Simplify Harbor setup

We configure Harbor with OIDC. Currently that requires an init container to call a Harbor API, which feels brittle. This task consists in investigating and upstreaming OIDC configuration of Harbar via a ConfigMap.
Please follow how the POSTGRESQL_HOST variable is being configured, and try to mimic that for OIDC variables:

It is set in a ConfigMap: https://github.com/goharbor/harbor-helm/blob/1acd0c7ef61c22ea90c877735517c5e6b7f10b17/templates/core/core-cm.yaml#L17
That ConfigMap is exposed via environment variables here: https://github.com/goharbor/harbor-helm/blob/1acd0c7ef61c22ea90c877735517c5e6b7f10b17/templates/core/core-dpl.yaml#L61
It is converted into an internal Harbor variable here: https://github.com/goharbor/harbor/blob/master/src/common/config/metadata/metadatalist.go#L107

The same could be done for OIDC variables. My understanding is that we "only" need to add them an EnvKey attribute here:
https://github.com/goharbor/harbor/blob/master/src/common/config/metadata/metadatalist.go#L136
Acceptance criteria:

I can use the upstream Harbor Chart and upstream Harbor image to setup OIDC via values.yaml in a robust and easy-to-understand manner.

Looking into this I quite quickly realized that this was not as simple as you described, so I found an similar issue and asked there. And it seems like there's a proposal on the way for this from the maintainers already https://github.com/goharbor/harbor/issues/13911

I think this is great! We engaged with upstream, learned about their roadmap and offered to help. I would mark this issue as "waiting for upstream" or close it. If we close it, we can either reopen it later, or open a new issue linked to the present one. Any preference @Xartos ?

I think we can close this one and once the proposal is in, we can create a new issue.

Perfect! Please do so. Just to make sure we don't miss this, I subscribed to the upstream thread as well.


Snappy compression failing for AWS Elasticache Memcached (Query Frontend)

Thanos, Prometheus and Golang version used:
v0.18.0

Object Storage Provider:
S3
What happened:
We are using AWS Elasticache Memcached as Thanos Query frontend. When we try to use snappy cache compression using the flag

--cache-compression-type=snappy

the cache is getting corrupted.
What you expected to happen:
snappy compression is expected to work as this is supported for Memcached.

Not able to reproduce the issue once I restarted the Memcached cluster to clear the cache.

I think this is valuable to investigate further. Thanos does a isDiffVarintSnappyEncodedPostings to check if an item is compressed. So even in the case that non-compressed items are posted, snappy encoding is enabled, it should be able to handle a mixture of items.


Configure ExternalTrafficPolicy field on auto-generated Services

What would you like to be added:
It would be nice if the Services generated by the creation of a Gateway could be configured with the externalTrafficPolicy field from the existing Services spec
Why this is needed:
At the moment it is required to deploy a Gateway, and then patch the Service created by it with this field as documented at https://istio.io/latest/docs/tasks/security/authorization/authz-ingress/#network
Not acting on this forces us to patch the service after every deployment--which is an anti-pattern. The Kubernetes model is to be declarative. Hack-patching imperatively after every apply is definitely not.
kubernetes-sigs/gateway-api#196 mentions this issue as being unsatisfactory, but was closed without providing a solution for this.
Note that this was originally opened as kubernetes-sigs/gateway-api#196 and every word there remains true. It was also closed without reason and justification, with a long rant about how it was too old and you're doing more than services now -- which I don't see how this relates?  Since discussion was abruptly closed by @youngnick when he wasn't reading what was written about the need, but instead going off the ranch on some other topics I will try to address what he said there:

Okay, your requirement is not externalTrafficPolicy: local then

My requirement is exactly that. I'm open to other solutions, and would use them if available and capable of meeting the need but at this time this is the only working solution. It's required to use Istio with a local LB, as shown in the documentation link.

presumably for layer 4 connections, since it's completely possible for HTTP connections using X-Forwarded-For or similar mechanisms.

No. The original request and this request is for this field to be set on the Service automatically generated by creation of the Gateway resource.  The X-Forwarded-For field can be set by an attacker, and often contains garbage.  This solution is the only viable mechanism, as proved by Istio documenting it as such plus dozens of posted issues/threads/etc.

The API is no longer called the "Services API" for a reason - because we decided to make the constructs more general than only Kubernetes Services. It's called the Gateway API now because we intended there to be implementations that don't use a Service for describing how Layer 4 traffic gets into a cluster. That's what Gateways and Listeners are for.

Um, fine for whatever purpose those things are used for?  But when I use the currently Gateway resource, it generates a Service. And there is no way to set this field on that service without either:

a post-deployment operation to patch the generated Service
a cluster-wide admissions controller that patches the generated Service

trying to reopen a three year old issue that proposes a solution, not a problem, is not a productive way of engaging with the community.

#196 described a problem, @youngnick simply decided it wasn't really a problem in his mind.  If you want to say that there's a better way to do this than setting that field, then document the solution rather than rudely insist that the need has already been met, when all documentation says otherwise.

While I think that perhaps the level of ad-hominem in this issue statement could be a bit lower, I'd like to thank you for opening a new issue, so that we can discuss what you're asking for in the current context. Perhaps in the future I would encourage you to limit the use of words like "rudely" and so on though.
For more context, in general we prefer to have new issues logged rather than re-opening old issues because it ensures that the current context is correctly handled, which is particularly relevant in this case.
I also note that the comments you're quoting from come from #451, not #196 (this took me a bit to trace the reference, sorry).
As I said above, your point is fair - having to manually patch a resource after it's generated by an implementation does suck, and it's why we introduced the infrastructure stanza in Gateway in GEP-1867, and added support for labels and annotations in GEP-1762.
However, in our upstream API review, we got a lot of pushback on having this level of config at the Gateway level, with the suggestion being that implementations should use multiple GatewayClasses that each have a parametersRef set that specifies the externalTrafficPolicy somehow.
So, a summary:

Is this a real issue? Yes, I agree that it is. Having to patch objects after they're created is not declarative and a bad place to be.
Do we have a place where we might put this configuration? Yes, we need to decide if this configuration meets the criteria for inclusion in the infrastructure stanza. (Which in itself involves defining the criteria and future shape of that stanza. Because this is a spec that we anticipate will live for a long time, we don't want to rush into something that will have us end up in an even worse place than we are now).
Could this whole discussion have been had in a more polite and constructive manner? Absolutely.

On the last item, reading over my comments on #451, I'll own that I could perhaps have been a bit more gentle with my phrasing. But for the future, I would suggest that coming off as making demands (which is absolutely how I read the responses on #451), and then writing a pretty accusatory issue description is not conducive to a constructive discussion.

@jorhett The way you've written this issue feels unnecessarily harsh. Please remember the Kubernetes Code of Conduct, specifically:

Demonstrating empathy and kindness toward other people
Being respectful of differing opinions, viewpoints, and experiences

I'd also mention that many of us are spending personal time to work on this project. Please understand that there is not infinite time to address every possible feature request.
Although you're correct that many implementations currently create a Service of type LB as part of their implementation of a Gateway, that is an implementation detail, it is not actually part of Gateway API. For example, some implementations don't use LB Services today, and in the future, implementations may create L4 Cloud Gateways instead of LB Services. As @youngnick has pointed out, it's more helpful to describe the kind of behavior you'd like a Gateway to have instead of how that would be implemented (ExternalTrafficPolicy=Local).
With all that said, if you want to proceed with this, please respectfully open an issue that describes the features you actually want to be configurable. ExternalTrafficPolicy=Local is really two things:

Route traffic to nodes with a healthy endpoint (avoid double hop)
Preserve Source IP

It's not clear from this issue which of those features you want and why. It's most helpful for us to understand the broader use case so we can avoid getting stuck with existing constructs.


StandardError: An error has occurred, this and all later migrations canceled:

undefined method `json' for #<ActiveRecord::ConnectionAdapters::TableDefinition:0x005588484c5998>
Did you mean?  JSON
/home/siswa/Documents/Projects/Rails/cmskms/db/migrate/20160727081536_create_kms_settings.kms.rb:5:in `block in change'
/home/siswa/Documents/Projects/Rails/cmskms/db/migrate/20160727081536_create_kms_settings.kms.rb:4:in `change'
-e:1:in `<main>'
NoMethodError: undefined method `json' for #<ActiveRecord::ConnectionAdapters::TableDefinition:0x005588484c5998>
Did you mean?  JSON
/home/siswa/Documents/Projects/Rails/cmskms/db/migrate/20160727081536_create_kms_settings.kms.rb:5:in `block in change'
/home/siswa/Documents/Projects/Rails/cmskms/db/migrate/20160727081536_create_kms_settings.kms.rb:4:in `change'
-e:1:in `<main>'
Tasks: TOP => db:migrate
(See full trace by running task with --trace)

Here is a Gemfile.
Any suggestion?

The Gemfile.lock.

Hello, @andisugandi. Glad you're trying KMS, going to add more detailed documentation soon.
What database adapter do you use? KMS expects using PostgreSQL database

My (silly) mistake, forgot to make the database before following the guide.
The issue is now solved.
Thank you, @GarPit and team.


Fix for #1627

Description
[Describe what this change achieves]
Fix for https://github.com/opensearch-project/security-dashboards-plugin/issues/1627
Category
Bug fix
Why these changes are required?
Locally setup saml
What is the old behavior before changes and new behavior after changes?
old behavior was buggy
Issues Resolved
[List any issues this PR will resolve (Is this a backport? If so, please add backport PR # and/or commits #)]
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List

[Yes] New functionality includes testing
[No] New functionality has been documented
[Yes ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
cc: @anijain-Amazon

@devardee to add to @peternied's comment, could you please update the PR title/description to be more detailed?

lets update the saml test cases to make sure this scenario is handled correctly with this change.

@peternied can you please guide me on what kind of testing needs to be added here ?
@DarshitChanpura the linked GH issue is very descriptive, can I copy/paste from there ?

I see there are end-to-end tests here, however, I am unable to run them locally, can the team pls help me on setting up my IDE, so that I can added new tests

@devardee To run those tests:

Create a local distribution of core and install the security plugin with the demo configuration
Replace the demo configuration's config.yml with the config.yml in this workflow.
Start OpenSearch
Clone OSD and also clone the security-dashboards-plugin in the plugins folder of OSD
Configure config/opensearch_dashboards.yml with the values present in this workflow
Start Dashboards
In a third terminal window navigate to OpenSearch-Dashboards/plugins/security-dashboards-plugin and run the command from this line: yarn cypress:run --browser chrome --headless --spec "test/cypress/e2e/saml/*.js"

@DarshitChanpura the linked GH issue is very descriptive, can I copy/paste from there ?

I've made changes to the description. Feel free to correct if anything is missing.

@peternied , @DarshitChanpura  I have added IT. Please take a look

the newly added test is failing in multi-auth test workflow 😔, can anyone please help me out on why that particular test is failing.
Not able to understand the failure from logs :
AssertionError: expected null to exist
      at Context.eval (webpack://opensearch-security-dashboards/./test/cypress/e2e/saml/saml_auth_test.spec.js:134:46)

@devardee I made some slight modifications to the test to first login, then to use cy.request instead of cy.visit like this:
it.only('Login to Dashboard with Goto URL', () => {
  localStorage.setItem('home:newThemeModal:show', 'false');

  cy.visit('http://localhost:5601/app/opensearch_dashboards_overview', {
    failOnStatusCode: false,
  });

  samlLogin();

  cy.shortenUrl(SHORTEN_URL_DATA, 'global').then((response) => {
    const gotoUrl = `http://localhost:5601/goto/${response.urlId}?security_tenant=global`;
    cy.request(gotoUrl);
    cy.getCookie('security_authentication').should('exist');
  });
});

cy.request will make a request to the shortened URL using the cookies currently stored in the browser and those cookies are set in the browser after login.

@cwperks , thanks for the help 😊. All the tests passed locally. Hope it pass in the github CI as well

@devardee Did you try the suggestion from this comment? Login first, then navigate to shortUrl?

Hi Team, pls take a look, github ci has passed
cc: @cwperks

It would be awesome if this could make it into 2.12


Improved docker image

Stop mounting entire working directory as volume
Only mount datasets as a volume (and maybe nf-ncov-voc submodule)
Copy remaining minimally required files directly to image
If we are going to copy nf-ncov-voc to the image, instead of mounting it as a volume, we should not move its executable to a directory requiring root permissions

Changes made:

Replace conda base image with python image, and also transition from using conda to docker for nf-ncov-voc calls
Copy remaining minimally required files directly to image
Only bind mount docker socket, datasets, and nf-ncov-voc submodule

Some issues:

Need docker socket proxy
README out of date
If you run a docker instance, upload a file, and then later try to upload a file without running a docker instance, you need to run following command in nf-ncov-voc directory:
sudo rm -r results work .nextflow .nextflow.log* capsule  framework  plugins  secrets  tmp
This is because your host will not have permission to execute these files generated within the previously deployed containers


Single Dataset Globus Transfer

Comparable to wget  give a Globus option for Globus-enabled datasets.
This feature would link to app.globus.org with the dataset brought into view at the correct endpoint and the user takes it from there.

Updated to mention a 2-prong approach

[ ] Schedule a transfer on click, provided a destination collection/path has been established
[ ] bring the user to the globus file-manager (this is either completing a template or grabbing a pre-populated url from the record)


Slowdown the admin services due to the database call to update the user PermissionTree.

Description:
Slowdown the admin services time to time. this is due to the database call made to update the user PermissionTree on each cache invalidation.
Suggested Labels:
N/A
Suggested Assignees:
N/A
Affected Product Version:
APIM 2.0
APIM 2.1
OS, DB, other environment details and versions:
N/A
Steps to reproduce:
Calling any admin service with concurrency will hit the database for each call [1]
[1] https://github.com/wso2-support/carbon4-kernel/blob/b19f6ec37167af7a5d5d2c2716ae91f80a09a37f/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/authorization/PermissionTree.java#L963
Related Issues:
N/A

This fix is available since 4.4.40
https://github.com/wso2/carbon-kernel/commit/bef576113f61d1f8788bb4d791a9a4e8406c85ac

Without this fix, if the permission cache is expired and multiple threads access this method simultaneously, all the threads will update the permission tree by connecting to the DB. After this fix, only one thread will go inside the synchronous block and update the cache by connecting to the DB. Other threads will retrieve the cached entry.


Please backport 5aad0a0b (symlinked content)

The last update broke my symlinked theme file. I saw that you fixed this on the master branch already (5aad0a0b99ec977b29a2b940fc5461b7f19ee4c2), but the betterdiscord-rauenzi-git AUR package uses the stable16 branch.

That's not backporting, stable16 is what everyone use's for their injection. There is currently an issue with symlinked files for everyone, symlinked folders (like the plugins and themes folder) are still fine

As I said there was an issue still with symlinked files, the commit I pushed 13 minutes ago however should fix that. You will need to reload for the changes to take effect and ensure that it is running that commit.

I tried ctrl+r and also restarting discord completely, but the symlinked theme file does not show up in my preferences.

what does $(script[src*="main.min.js"])[0].src produce in console?

$('script[src*="main.min.js"]')[0].src
"https://cdn.staticaly.com/gh/rauenzi/BetterDiscordApp/9d1613466e210297a6169f0e3984fe0da0bce9de/js/main.min.js"

hmm weird, I will do some additional testing in that case.

Are the symlinked files not loaded on startup?


While loop not working properly in JSBIN javaScript external but works well when embedded/inline

ar count=1;
var superScript;
while(true){
if(count === 1){
superScript="st";
}
else if(count===2){
superScript="nd";
}
else if(count===3){
superScript="rd";
}
else{
superScript="th";
}
var password = prompt("Password please");
if(password!=="Rugera"){
document.write("No access granted "+count+superScript+"");
count++;
if(count===5){
document.write(count-1," attempts made. No more goes. Bye!")
break;
}
}
else{
document.write("At ",count+superScript, " attempt access granted. Well done");
break;
}
}

Your code was kinda hosed. This is how it should be written:
var count = 1;
var superScript;
while (true) {
  if (count === 1) {
    superScript = "st";
  } else if (count === 2) {
    superScript = "nd";
  } else if (count === 3) {
    superScript = "rd";
  } else {
    superScript = "th";
  }

  var password = prompt("Password please");
  if (password !== "Rugera") {
    document.write("No access granted " + count + superScript + "");
    count++;
    if (count === 5) {
      document.write(count - 1, " attempts made. No more goes. Bye!")
      break;
    }
  } else {
      document.write("At " + count + superScript, " attempt access granted. Well done");
      break;
  }
}

Thank you remy. Did you try it in JSBIN? My problem is that it runs only once and breaks. But if I use it in other environments it works perfectly. Thank you again


Error processing mechanism

As a user
I want all the async tasks to be guaranteed processed with the processing result
So that even if any server error happened during the processing, it can be processed later and that data will keep in consistent state.

[ ] Case:  Something wrong with PRM, got error "description": "Employee with such ID is not found"


Add basic commands for interacting with contracts

Description
Add basic commands for tf-grid-cli to interact with contracts as a quick alternative to the GUI or the clients.
For example:

Get all contracts
Get a specific contract details
Cancel contract(s)

Finished get contracts cmd:
$ bin/tf-grid-cli get contracts
5:53PM INF starting peer session=tf-144830 twin=192
Node contracts:
ID       Node ID    Type       Name               Project Name
50418    13         network    testingnetwork     testing
50419    13         vm         testing            testing
50420    34         network    testing2network    testing2
50421    34         vm         testing2           testing2

Name contracts:
ID       Name
50423    se7r

Deployed a vm and a name gateway

Get contracts output:

5:13PM INF starting peer session=tf-1184566 twin=81
Node contracts:
ID       Node ID    Type            Name           Project Name
50977    21         network         vm1network     vm1
50978    21         vm              vm1            vm1
50980    14         Gateway Name    gatewaytest    gatewaytest

Name contracts:
ID       Name
50979    gatewaytest

get contract 50977 output:

5:14PM INF starting peer session=tf-1185180 twin=81
5:14PM INF contract:
{
        "contract_id": 50977,
        "twin_id": 81,
        "state": "Created",
        "created_at":<PHONE_NUMBER>,
        "type": "node",
        "details": {
                "nodeId": 21,
                "deployment_data": "{\"type\":\"network\",\"name\":\"vm1network\",\"projectName\":\"vm1\"}",
                "deployment_hash": "21adc91ef6cdc915d5580b3f12732ac9",
                "number_of_public_ips": 0
        }
}

cancel contracts output:

5:17PM INF starting peer session=tf-1185964 twin=81
5:17PM INF contracts canceled successfully

All worked fine
Problem

I tried to use command get contract without an ID and it panicked

To add

Please add all supported commands in the readme

All verified except:

I tried Cancel contracts without any flags, and it returned an info instead of error

11:20AM INF please specify contracts to cancel or use -a to cancel all

Verified.


Improve default security configuration for production environment

Is your feature request related to a problem? Please describe.
We currently don't configure anything special related to security, we just let the default Django configuration and assume it is to the project maintainer to decide and implement about security
Describe the solution you'd like
We should enable the Django security middleware in the production settings.
It just needs some test for CMS edition interface and CKEditor because frontend can sometime rely on some request/response behaviors that may be blocked by some security settings (like iframe forbidding).
There is also some concerns about the settings not managed by the security middleware:

SESSION_COOKIE_SECURE to force usage of HTTPS cookie only;
and maybe SESSION_EXPIRE_AT_BROWSER_CLOSE for age expiration on session cookie, but some users could find this painful;
CSRF cookie have almost the same settings also;

Describe alternatives you've considered
Just notify about the Security middleware in documentation so project maintainer is aware of it.

Middleware documentation needs to be readed comprehensively by a sysadmin before so we are sure it works well on our infrastructure and so we can advise for some settings and provide some possible documentation.

Surprisingly enough, the security middleware since Bireli 0.2.0 ...
The following settings SESSION_COOKIE_SECURE, SESSION_EXPIRE_AT_BROWSER_CLOSE, CSRF_COOKIE_SECURE have been turned on in production settings in v0.4.0


[azeventhubs] UX study prep for package

Make it the usage of context more prominent. See if that helps users figure out they need to pass it. Docs and examples.
Fix event hubs examples to pass in context with a timeout.

Done - examples with receiving always create a context now, with a timeout of 1 minute and a little comment blurb above it telling you why and the comment for ReceiveEvents() contains a bit explaining how you can control its lifetime.


HOWTO: Update CNI plugin

Is this a request for help?: YES

What version of acs-engine?:
Version: canary
GitCommit: 5b5e8bf
GitTreeState: clean

Orchestrator and version (e.g. Kubernetes, DC/OS, Swarm)
Kubernetes 1.7.5
How can I update the Azure CNI plugin version in my cluster?

We don't currently expose it as a configuration parameter. See:
https://github.com/Azure/acs-engine/blob/master/pkg/acsengine/defaults.go#L32
Are you looking to use a newer version than v0.91?

@jackfrancis  No, I'm trying to upgrade the CNI plugin in a cluster that was created with an older version of acs-engine (and therefore uses an older version of the CNI plugin).
Trying to get the fix for https://github.com/Azure/azure-container-networking/issues/76.

There is no supported way to do this. You could manually perform the steps here, using the desired CNI artifacts:
https://github.com/Azure/acs-engine/blob/master/parts/kubernetesmastercustomscript.sh#L168
@JackQuincy Just in case, I'll ask: would a scale/upgrade operation do this for @rocketraman on an existing cluster using a newer version of acs-engine. I think not, but perhaps adding more nodes to an existing cluster w/ a fresher config would give the desired azure CNI presence, after which the old nodes could be deprecated.
Either way it's rather manual!

@jackfrancis Replacing the cni binaries with the newer version seems easy enough. Do I need a reboot of each node after doing that?

@rocketraman replacing binaries would work. Reboot should not be required. However, to reclaim the lost ip-addresses, you will need to fix the CNI state. It is easy to clean up the state completely and start from fresh. However, if there are some existing pods on the node, then the state clean up will need to accommodate for those. I think its better to create new cluster/nodes.

@sharmasushant :  If I create new nodes, migrate all workload to them, and then remove the old nodes, will the necessary CNI state cleanup happen?
@jackfrancis Is the best way to clean this up with acs-engine to create a new node pool, migrate workload to it, then scale down the old node pool to 0 nodes?

@JackQuincy The scaling documents here suggest there is no scale-down. And @rocketraman's mention of creating a new node pool suggests there's an idiomatic workflow to do that w/ acs-engine. Is a az group deployment create against the same resource group with a modified api model (w/ the new agent pool) the way to do that?


docs: [IAC-2029]: Add custom images doc

Description

Add new custom images doc explaining how to create and use a custom image with your Terraform plugin steps.
JIRA

PRs must meet these requirements to be merged:

[x] Successful preview build.
[x] Code owner review.
[x] No merge conflicts.
[x] Release notes/new features docs: Feature/version released to at least one prod environment.

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step behind the Harness VPN and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app. Current Draft URL is: https://66604113cab856f6005f546b--harness-developer.netlify.app

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step behind the Harness VPN and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app. Current Draft URL is: https://66604ae64f2a36fdb6f9bb26--harness-developer.netlify.app

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step behind the Harness VPN and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app. Current Draft URL is: https://66604fb4246f6f01b69b7a14--harness-developer.netlify.app

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step behind the Harness VPN and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app. Current Draft URL is: https://66617345b4321a794707d8f5--harness-developer.netlify.app


Normalise pipeables

Consider changing the markup-html, pug-render and highlighting pipeables to work on a single page, managing the pages mapping in the pipe.

These won't be interchangeable where they use context (as press and compile context are different) but even when not, operating on a single page means the iteration over all pages in the compile pipe only needs to happen once for all plugins.


Add support for Drupal

Add warden environment files for Drupal support.

Before I merge this, can I get your approval on my additions?

[ ] https://github.com/wardenenv/warden/pull/646/commits/e4eacf2bff42acb103d143786783194c27bd932e
[ ] https://github.com/wardenenv/docs/pull/9

@navarr  I took a quick glance and it all looks okay to me.


Watchlist final

This PR captures:

Logging Client to do specific event logging (would like to refactor this further when we add more logging, since I think there should be a cleaner way to do this, but I felt like this was good for now)
App Insights Client enhancement to add custom metrics to events, exceptions
Constants file for app insights logging
Watchlist app insights logging (exceptions, events)
Service bus streaming controller to send service bus messages for events for blacklist, sitesettings, streams and watchlist changes
microsoft translator bug fix with translationSvcToken casing and logging for that
add remove keywords service edits for ddl changes

TestCases.docx
corresponding test cases


enhancing test, docker, and release actions

[x] I have added the tests to cover my changes.
[x] I have updated the documentation and CHANGELOG accordingly.
[x] I have read the CONTRIBUTING document.

Summary of changes:

Functional tests do not run on PRs
Tests can be run manually with a workflow_dispatch trigger provided a commit SHA
docker.yml is now only for dev work and builds an image from a commit SHA using a manual trigger
release.yml includes a step for Docker

Other changes included in this PR:

Makefile was removed from the codebase
Dockerfile was modified to include the required rsync dependency


Crash when receiving a Twilio Call while you are in native call

Description
When you are in a standard pstn call and you receive a Twilio call, the call gets terminated and a dialog saying "Call Failed" appears. First, I thought this was some issue in my app but it's reproducible with the quickstart repository.
Steps to Reproduce

Make a PSTN call to a PSTN number
Call the placeCall endpoint to initiate a call to the device while in call

Code
No code apart from the Quickstart code is necessary to reproduce this issue
Expected Behavior
The PSTN call should continue, a dialog should appear to let you choose between calls
Actual Behavior
The active PSTN call gets ended, and the Twilio call is not displayed
Reproduces How Often
100%
Logs
A callservicesd crash is generated:
callservicesd  10-24-19, 4-08 PM.log
Versions
All relevant version information for the issue.
Voice iOS SDK
5.0.0
Xcode
11.1
iOS Version
13.1.2
iOS Device
iPhone SE
iPhone 11 Pro

@MarcoCanalcar Are you still running into the crash? We haven't seen the crash in our validation.

Hi @MarcoCanalcar
Hope things are well. I am closing this ticket assuming the original issue has been resolved by the iOS updates. Please feel free to reach out to us.
Cheers,
bobie


Help using this please

Hi,
I would love to be able to use latex in slack but I'm not familiar with how to install things in slack, so don't understand the instructions under "how to use". Are you able to give me more specific instructions on what to do? This would be very much appreciated, thanks!

This uses something called node.js which lets you run javascript. Once you install nodejs, go to the directory you placed the bot.js file and use those commands from terminal to install the required dependencies. You need to have a slack API token either of yourself or of a bot from the Bots slack app so that this program can make posts on your slack.

Thanks so much, that worked!


Avoid continually removing the stale label using a keep label.

This could be useful to avoid continually removing the stale label by using a keep label.
Example can be found on #292
If you merge this pull request you can then add the keep label and forget about contanstly removing the stale one :)

Hi @sblantipodi are you able to sign the ECA at: https://accounts.eclipse.org/user/95367/eca/3.1.0

Hi @sblantipodi are you able to sign the ECA at: https://accounts.eclipse.org/user/95367/eca/3.1.0

Done It, thanks :)

Hi @sblantipodi are you able to sign the ECA at: https://accounts.eclipse.org/user/95367/eca/3.1.0

@karianna Done It, thanks :)


chore: remove SplitIf.ext cache

Incompatible as is with parallelism; let's first check if it has any impact at all

!bench

Mathlib bench is insignificant as well: http://speed.lean-fro.org/mathlib4/run-detail/736f134c-465b-45d6-b096-bb5f2f5ac81c


Hide menu bar by default on Windows and Linux. Alt toggles it.

Looks great! I didn't know electron had a preference for this.
I'm wondering, though, how many people know that alt shows the menu bar? Chrome and Firefox have a side menu, so you don't need the menu bar to access features, but in Min a lot of features are only available in the menu bar. Is this somethig a lot of apps do on those platforms?

Alt or Ctrl+M to toggle menu bar is the standard, though most applications have the menu bar shown by default. That said, other electron applications usually have autohide off by default, and allow one to toggle it in the View menu.

OK, I'll merge this and file an issue about adding an option: #74.

Doesn't seem to be working on Windows 10 x64


Upgrade eslint

The current version of eslint is from March 2016. Please upgrade to the latest version possible so we can get our hands on all those new es6 rules.

No thanks. Read https://nodesource.com/blog/semver-tilde-and-caret/ and learn how ^ in package.json works.

@shinnn Thank you for the clarification. What's the easiest way to upgrade transitive dependencies? Is npm-shrinkwrap the only way?

Use https://stackoverflow.com/ for non project-related questions.

It doesn't make much sense that you wouldn't want to make this package as user friendly as possible. If there is more functionality available, why not upgrade? It's a single line change.

@BreaGaudioso Read https://github.com/adametry/gulp-eslint/issues/189#issuecomment-271485125 again.


Config handles minimum_candidate_stagers set to < 1

If minimum_candidate_stagers is set to a value less than 1 in a manifest, reset it to the default value.

[X] I have viewed signed and have submitted the Contributor License Agreement

[X] I have made this pull request to the master branch

[X] I have run all the unit tests using bundle exec rake

[X] I have run CF Acceptance Tests on bosh lite

[#119362989]
Signed-off-by: Swetha Repakula<EMAIL_ADDRESS>

Hello @ScarletTanager,
Just curious, is there a reason that you need an extra check here for the minimum_candidate_stagers and nothing else?
Before we merge this, can you change the description of this property to include a description of this new behavior?
Thanks,
@adowns01 && @rizwanreza

@adowns01 - I'll definitely see about updating the property description.  We only need the extra check for minimum_candidate_stagers because that is the only property that really matters for the math done by the CC in determining which DEAs may be used to stage an application.  The current minimum size of the candidate pool is hard-coded to 5 - so if you have 5 or fewer DEAs in your deployment, it is really just selecting one at random, to some extent.  This addresses a bug identified by a user with a small deployment who noticed that it was still possible for his most loaded DEAs to be targeted for staging.

Thanks for the commit! It has been merged now.


How make Row width the same as the widest for all other Rows

Hi,
I have a scenario that does not seem that complex but I am unable to get it to work.
I have a column that contains two rows:
The first row contains icons where you select an option on a scale say from 1 to 4.
The second row contains two text strings explaining the min and max value of the scale, say Sad vs Happy.
What I want to accomplish is that the labels on the second row are aligned to the far left and right of the row above.
I know I can put the Column in a fixed width Container and then use MainAxisSize.max on each Row but that would require me to compute the width of the Row with the Icons and that is not a viable option since the number of icons can change and their sizes as well.
Is there a good way to accomplish this? I tried Expanded on the Row, Align etc but none has provided a solution.
This is a minimal example of what I have so far:

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
        visualDensity: VisualDensity.adaptivePlatformDensity,
      ),
      home: MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  MyHomePage({Key key, this.title}) : super(key: key);
  final String title;

  @override
  _MyHomePageState createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  final List iconList = [
    Icons.cloud,
    Icons.cloud_queue,
    Icons.wb_sunny,
    Icons.star,
  ];

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Padding(
      padding: const EdgeInsets.symmetric(vertical: 50.0, horizontal: 20.0),
      child: Column(
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
          Text(
            'How do you feel today?',
          ),
          SizedBox(height: 24.0),
          Container(
            color: Colors.lightBlue[100],
            child: Column(
              children: <Widget>[
                Row(
                  mainAxisSize: MainAxisSize.min,
                  mainAxisAlignment: MainAxisAlignment.center,
                  children: _buildIconBalls(),
                ),
                SizedBox(height: 8.0),
                Row(
                    mainAxisAlignment: MainAxisAlignment.spaceBetween,
                    children: [
                      Text('Min value'),
                      Text('Max value'),
                    ]),
              ],
            ),
          ),
        ],
      ),
    ));
  }

  List<Widget> _buildIconBalls() {
    return iconList
        .asMap()
        .map(
          (index, icon) => MapEntry(
              index,
              GestureDetector(
                onTap: () {},
                child: Container(
                  decoration: BoxDecoration(
                    shape: BoxShape.circle,
                    color: Colors.white,
                  ),
                  child: Padding(
                    padding: const EdgeInsets.all(12.0),
                    child: Icon(
                      icon,
                      size: 26,
                      color: Colors.black87,
                    ),
                  ),
                ),
              )),
        )
        .values
        .map((e) => [e, SizedBox(width: 15.0)])
        .expand((element) => element)
        .toList()
          ..removeLast();
  }
}

The result is this:

But I want it to be like this:

i think this should be included into https://flutter.dev/docs/development/ui/layout

Hi @TahaTesser
I am not able to find a solution for this specific issue there, are you thinking about a chapter in particular or did you mean that info could be added there?
My challenge is that I want to use:
mainAxisSize: MainAxisSize.min,
for the first Row and then have the second row adapt to the size of the first row so that the two labels are laid out with spaceBetween but respecting the size of the widest of the Rows in the column.
If the Column would use the full width it would not be an issue since both rows would span the whole width of the column but here the first Row should only use the space needed, hence the MainAxisSize.min.

Hi,
I found the solution, the here: https://medium.com/flutter-community/flutter-layout-cheat-sheet-5363348d037e
The IntrinsicWidth() did exactly what I wanted.

Hi @ansolantz
I have tried IntrinsicWidth() at first but this row wasn't getting any space in between
   Row(
                    mainAxisAlignment: MainAxisAlignment.spaceBetween,
                    children: [
                      Text('Min value'),
                      Text('Max value'),
                    ]),
              ],

How do you solve that?
Please provide code sample, it might be worth adding to official doc itself
Thank you

Hi @TahaTesser,
I wrapped it around the Column and it works fine with space between on the second row.
Here is my code:
Widget build(BuildContext context) {
    return Scaffold(
      body: Container(
      alignment: Alignment.center,
      padding: const EdgeInsets.symmetric(vertical: 50.0, horizontal: 20.0),
      child: Column(
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
          Text(
            'How do you feel today?',
          ),
          SizedBox(height: 24.0),
          Container(
            color: Colors.lightBlue[100],
            child: IntrinsicWidth(
              child: Column(
                children: <Widget>[
                  Row(
                    mainAxisSize: MainAxisSize.min,
                    mainAxisAlignment: MainAxisAlignment.center,
                    children: _buildIconBalls(),
                  ),
                  SizedBox(height: 8.0),
                  Row(
                      mainAxisAlignment: MainAxisAlignment.spaceBetween,
                      children: [
                        Text('Min value'),
                        Text('Max value'),
                      ]),
                ],
              ),
            ),
          ),
        ],
      ),
    ));
  }


First run page

#109

@spectralsun Looks good, I'll write new content.
@irdan Can you record a new GIF with the message "Using Privly means websites won't read your messages." The example GIF should not target a single website and I think more GIFs will clutter the page.


Organization owner should be able to view generic stats for effort subscriptions

It would be nice to have a page that provides subscription information for a given event group. For example, x subscriptions relating to y efforts (how many by email, how many by text) and z messages sent to date related to the event.
This is probably a new view accessed from the Admin menu.
Owners and Stewards should have access to this information.

This was addressed by 0c2485019eb19438b34ca9196ff83134de36a786


Organize chapters

I want the chapters to be more useful as a reference.
User story
When user comes to the style guide, they have certain style question in mind.
I want to organize the table of content so that the user knows right away where to look for answers.

Make this an on-going target and part of the book.


Change scroll behavior will broken markdown mode.

Describe the bug
Set
* {
    scroll-behavior: smooth;
}

then, change to markdown mode, editor will auto scroll infinity.
To Reproduce

Expected behavior
Set scroll-behavior: smooth; the markdown mode work well.
Screenshots

Additional context
Long text content more obvious

Looks very funny.

@cary-hu What is your browser? I want to know your environment information.

@cary-hu Sorry, one more, please. What's your OS? Is it windows?

Yes
Edition	Windows 11 Pro
Version	22H2
Installed on	2022/5/7
OS build	22616.1
Experience	Windows Feature Experience Pack 1000.22632.1000.0

@cary-hu Thank you! Actually, I use macOS and It works fine. I'll check it.