Merge lp://staging/~tealeg/landscape-client/allow-duplicate-keys-in-config into lp://staging/~landscape/landscape-client/trunk
Status: | Merged |
---|---|
Approved by: | Geoff Teale |
Approved revision: | 735 |
Merged at revision: | 731 |
Proposed branch: | lp://staging/~tealeg/landscape-client/allow-duplicate-keys-in-config |
Merge into: | lp://staging/~landscape/landscape-client/trunk |
Diff against target: |
142 lines (+73/-6) 3 files modified
landscape/deployment.py (+13/-2) landscape/tests/test_configuration.py (+4/-2) landscape/tests/test_deployment.py (+56/-2) |
To merge this branch: | bzr merge lp://staging/~tealeg/landscape-client/allow-duplicate-keys-in-config |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alberto Donato | Approve | ||
Chris Glass (community) | Approve | ||
Review via email: mp+187169@code.staging.launchpad.net |
Commit message
This branch catches ConfigObjError instances that are returned at the
end of parsing landscape-
(for example duplicate keys). Those errors are logged and the rest of
the configuration is recovered from the error and passed back. In
this way we emulate the tolerant behaviour we had prior to the switch
to ConfigObj.
Description of the change
This branch catches ConfigObjError instances that are returned at the
end of parsing landscape-
(for example duplicate keys). Those errors are logged and the rest of
the configuration is recovered from the error and passed back. In
this way we emulate the tolerant behaviour we had prior to the switch
to ConfigObj.
This looks good, however I'd like to add some tests to make sure we describe the behavior well:
[1]
Please add a test with keys before and after the duplicate entry, to assert these are not lost (or how they are lost).
Consider:
[client] example. com/message- system
url = http://
computer_title = frog
computer_title = flag
log_level = debug
What is config.log_level in this case?
[2]
What happens when more than one keys are duplicates? triplicates?
[3]
+ but the latest defined value should be used.
+ """
This contradicts the test, where the *first* defined value is used.
[4] config_ source, list_values=False,
+ config_obj = ConfigObj(
+ raise_errors=False)
Please add a comment explaining what raise_errors does (similar to the one about list_values)