Code review comment for lp:~radix/txaws/parameter-enrichment

Revision history for this message
Thomas Herve (therve) wrote :

[1] In _secret_convert_old_schema, you should pass item.name as name parameter to List, to keep compatibility.

[2] You also need to forward the optional parameter.

[3] Still in the sample, it seems you should forward the default value, and use [] otherwise as default.

[4] In _convert_nest_to_flat, you return as soon as you get a dict: if you have more params, they get ignore. You should probably call _result.update instead.

[5]
+ {'foo.1.bar': 'value',
+ 'foo.2.baz': 'value'}
+
+ to::
+
+ {'foo': {'1': {'bar': 'value'},
+ '2': {'baz': 'value'}}}

I think we talked about that, but what's the reason we don't convert this to:

{'foo': [{'bar': 'value'}, {'baz': 'value'}]} ?

FWIW, I discovered most of this running Landscape/Clouddeck. We can deal with some small incompatibilities, but it'd be nice to narrow them down. Thanks!

review: Approve

« Back to merge proposal