Merge lp://staging/~therve/landscape-client/juju-env-info into lp://staging/~landscape/landscape-client/trunk

Proposed by Thomas Herve
Status: Merged
Merged at revision: 606
Proposed branch: lp://staging/~therve/landscape-client/juju-env-info
Merge into: lp://staging/~landscape/landscape-client/trunk
Diff against target: 310 lines (+91/-68)
3 files modified
landscape/broker/registration.py (+25/-24)
landscape/broker/tests/test_registration.py (+53/-23)
landscape/message_schemas.py (+13/-21)
To merge this branch: bzr merge lp://staging/~therve/landscape-client/juju-env-info
Reviewer Review Type Date Requested Status
Björn Tillenius (community) Approve
Chris Glass (community) Approve
Review via email: mp+143452@code.staging.launchpad.net

Description of the change

The branch adds a new registration fields populated with the content of a file in the data path. I'm not super happy about it yet, but it feels it does the job.

To post a comment you must log in.
Revision history for this message
Chris Glass (tribaal) wrote :

Looks good, +1!

I hope goju will provide an API instead, it would feel "cleaner", but this will do nicely until then :)

review: Approve
Revision history for this message
Björn Tillenius (bjornt) wrote :

I think it's good enough, so I'm giving it a +1. As mentioned on IRC, I'd rather
see each key and value go in the message itself, rather than sending the JSON
blob. I'm not sure how easy it would be to extend our schema to be more flexible,
though. This is one way of allowing a schema to contain any number of optional
keys, but not a very pretty one.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: