Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SimpleInventory crashes with empty hosts file #826

Open
mpaccione opened this issue Oct 19, 2022 · 5 comments
Open

SimpleInventory crashes with empty hosts file #826

mpaccione opened this issue Oct 19, 2022 · 5 comments

Comments

@mpaccione
Copy link

Hi, your code doesn't allow for an empty hosts file.
I find this perplexing given that groups and default yaml files have this check?

 def load(self) -> Inventory:
        yml = ruamel.yaml.YAML(typ="safe")

        if self.defaults_file.exists():
            with open(self.defaults_file, "r", encoding=self.encoding) as f:
                defaults_dict = yml.load(f) or {}
            defaults = _get_defaults(defaults_dict)
        else:
            defaults = Defaults()

        hosts = Hosts()
        with open(self.host_file, "r", encoding=self.encoding) as f:
            hosts_dict = yml.load(f) // CRASHES : WHY IS THERE NO or {} HERE??? //

        for n, h in hosts_dict.items():
            hosts[n] = _get_inventory_element(Host, h, n, defaults)

        groups = Groups()
        if self.group_file.exists():
            with open(self.group_file, "r", encoding=self.encoding) as f:
                groups_dict = yml.load(f) or {}

            for n, g in groups_dict.items():
                groups[n] = _get_inventory_element(Group, g, n, defaults)

            for g in groups.values():
                g.groups = ParentGroups([groups[g] for g in g.groups])
@ubaumann
Copy link
Contributor

What is your use case where you need the SimpleInventory but empty?

@mpaccione
Copy link
Author

Hi @ubaumann

Initial setup and install of a product to have an empty yaml file for users.
Meaning that the main config.yaml references the file paths correctly and things are in the correct locations but it is empty for users to edit. A better process than having them manually create the files themselves and potentially become confused about other configuration and file path details.

What do you think? Is there a better way here given the exact intention?

@ubaumann
Copy link
Contributor

I would add a demo host to the configuration file. Mybe the localhost. Otherwise, you have to wait for the response of the maintainer on how he feels about a PR or implementing your inventory plugin.

I assume it also fails if your host file looks like this?

---
...

@mpaccione
Copy link
Author

Correct, it also fails.

Currently I am making a util function that gets the file path from the config yaml and then does a kinda of validation check on the hosts file.

I know the docs say hosts are mandatory so this might be a far fetched request but imo a crash is a really rough way to handle this.

@dbarrosop
Copy link
Contributor

My concern with not crashing and allowing an empty hosts file is that if a user makes a typo and points to a file that doesn't exist (i.e. pointing to hosts.yaml while they actually called it hosts.yml) the whole thing will fail silently. You could argue the same is true for groups and defaults but the main difference is that the need for an empty hosts file is an edge case (first time someone asks for this) while lots of people doesn't have groups and defaults.

What I'd suggest is the following; create your own Inventory plugin that inherits from SimpleInventory, before calling super do your check and return an empty inventory if the file doesn't exist or if it's empty. Otherwise call super and just let SimpleInventory do its job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants