Updated pagination for jmespath result_keys.
#232
Conversation
|
Looking, but have you seen the travis failure? We need to change the unittest import to |
| @@ -57,3 +59,35 @@ def remove_dot_segments(url): | |||
| output.append(url[:next_slash]) | |||
| url = url[next_slash:] | |||
| return ''.join(output) | |||
|
|
|||
|
|
|||
| def exp_set(source, expression, value, is_first=True): | |||
There was a problem hiding this comment.
Can we rename this to something more descriptive? It's hard to tell what this does based on the name. jmespath_expression_set, set_value_from_expression, expression_set_value, etc.
|
I think the approach looks good overall. In addition to adding the pagination configs, don't forget we'd also need updated unittests showing that result_keys work with jmespath expressions. Feel free to update the PR when you'd like me to take another look. |
|
|
Looks good!
| for vals in zip_longest(*iterators): | ||
| for k, val in zip(key_names, vals): | ||
| if val is not None: | ||
| response[k].append(val) | ||
| response.setdefault(k.expression, []) | ||
| response[k.expression].append(val) |
There was a problem hiding this comment.
Sorry, this was a miss on my part, we should not be setting any key names to k.expression, we should be using set_value_from_jmespath. For example if you run aws rds describe-engine-default-parameters --db-parameter-group-family mysql5.1 you'll see that the result is:
{
"EngineDefaults": {
"Parameters": []
},
"EngineDefaults.Parameters": [
{
"Description": "Controls whether user-defined functions that have only an xxx symbol for the main function can be loaded",
...
}
}
There was a problem hiding this comment.
Gah, sorry. :( I could get to this over the weekend, if that helps. The test I added is probably wrong too.
This is a WIP. I think the implementation is complete (unit tests are back to passing + additional tests), but we still need to update all the pagination configs. Before I add that to the PR, review of the approach/implementation would be appreciated.
@jamesls @danielgtaylor @garnaat