launchpad-reviewers team mailing list archive

[Martin Packman <martin.packman@xxxxxxxxxxxxx>]

Thanks for looking at this Raphaël.


> Not sure why you decided to do this in the view code instead of passing a
> 'constraints' parameter to get_nodes().  This way,
> get_available_node_for_acquisition could also pass on the 'constraints'
> argument to get_nodes().  Don't you think it would be better?

Probably, though there was a fair bit of back and forth already about how to spell this in the earlier merge proposals already:

<https://code.launchpad.net/~jameinel/maas/cpu_mem_tag_constraints/+merge/126863>

Provided the api and the ui can share the underlying logic sanely, I'm not too fussed. 

> This might be obvious but why can't this be shared with maasserver.api
> somehow?  I guess I'm probably missing some context here.

It's currently slightly different. Making it more similar and sharing the code is probably worthwhile and I'll poke that in a followup branch.
-- 
https://code.launchpad.net/~gz/maas/node_search_view/+merge/128296
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gz/maas/node_search_view into lp:maas.