Validate inventory file (#662) #1182
Conversation
| @@ -258,7 +270,7 @@ def make_inventory_from_files( | |||
| for hosts in groups.values(): | |||
| # Groups can be a list of hosts or tuple of (hosts, data) | |||
| hosts = _get_any_tuple_first(hosts) | |||
|
|
|||
| # TODO: Generator expressions are silently discarded (?) | |||
There was a problem hiding this comment.
I'll take a look at this.
pyinfra_cli/inventory.py
Outdated
|
|
||
| return all(isinstance(item, ALLOWED_HOST_TYPES) for item in value) | ||
| if not all(isinstance(item, ALLOWED_HOST_TYPES) for item in value): | ||
| logger.warning('Ignoring host group "%s". Host groups may only contain strings (host) or tuples (host, data).', key) |
There was a problem hiding this comment.
Should this be an error instead?
On one hand, an inventory file might contain other data structures since you can do whatever you want in an inventory file (similar to dynamic inventory in Ansible), so even this warning might be too verbose?
On the other hand, silently ignoring variables due to invalid types is also problematic (see #662).
Any thoughts?
There was a problem hiding this comment.
I think warning is the correct behavior for now - don't want to break anything existing but also need the warning so at least people will know!
There was a problem hiding this comment.
This looks great so far, thank you @simonhammes!
I think long term pyinfra should be moving towards inventory functions as the default to avoid the weird "list or tuple" thing for hosts/groups.
There's also potential for a DSL kind of inventory, but that's a future problem :)
No description provided.