Merge lp:~racb/maas/arch-constraint-wildcard into lp:~maas-committers/maas/trunk
Proposed by
Robie Basak
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Robie Basak | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 1226 | ||||
Proposed branch: | lp:~racb/maas/arch-constraint-wildcard | ||||
Merge into: | lp:~maas-committers/maas/trunk | ||||
Diff against target: |
188 lines (+96/-6) 4 files modified
src/maasserver/enum.py (+4/-0) src/maasserver/models/node_constraint_filter.py (+53/-1) src/maasserver/tests/test_api.py (+3/-3) src/maasserver/tests/test_node_constraint_filter.py (+36/-2) |
||||
To merge this branch: | bzr merge lp:~racb/maas/arch-constraint-wildcard | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeroen T. Vermeulen (community) | Approve | ||
Martin Packman (community) | Approve | ||
Review via email: mp+128513@code.launchpad.net |
Commit message
Add automatic architecture constraint wildcarding and arm alias
To post a comment you must log in.
Looks good.
The generate_ architecture_ wildcards is very cute, I'm not totally convinced it's needed versus just making someone type that out next to the arch enum, but as you'll be the one who has to add arm flavours it's fair enough to save yourself the bother. :)
Some small notes:
+ # Replace an alias with its value if it is an alias architecture_ aliases[ value]
+ try:
+ aliased_value = primary_
+ except KeyError:
+ aliased_value = value
This is not an exceptional circumstance, so I'd use the following (still non-recursive) spelling instead:
aliased_value = primary_ architecture_ aliases. get(value, value)
+ if aliased_value in (choice[0] for choice in ARCHITECTURE_ CHOICES) :
Rather than making a generator on the fly here to test membership, I'd do either: architecture_ wildcards and use the block below only CHOICES_ DICT in maasserver.enum as per other objects there and use `if aliased_value in ARCHITECTURE_ CHOICES_ DICT` instead.
* Add the full form as a mapping to a 1-tuple of itself in generate_
* Create an ARCHITECTURE_
I find test_generate_ architecture_ wildcards slightly hard to understand, a docstring, comment or more assertions might help.