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

Allow yaml extension for inventory and requirements #2225

Merged
merged 2 commits into from
Jul 11, 2022
Merged

Allow yaml extension for inventory and requirements #2225

merged 2 commits into from
Jul 11, 2022

Conversation

netsandbox
Copy link
Contributor

No description provided.

@netsandbox netsandbox requested a review from a team as a code owner June 28, 2022 07:32
@ssbarnea
Copy link
Member

Thanks for raising it, now we need to think a little bit about the implications.

The title is a little bit misleading because the tool does already recognize .yaml extension very well, is only the ansible specific config files which use only the default/documented extension. That was by design and not a mistake.

I am not sure we want to allow this because using these extensions will likely break LOTS of tools in the ecosystem, as most of them only recognize one filename.

We might require documentation proof that these extensions are allowed. The fact that they might happen to work, may not be enough. I mention this because recently we faced some other request about things that were never documented as being supported/allowed.

Keep in mind that it is summer, so we might need to wait a little bit more when we have to make a decision. Meanwhile you can configure overrides in your ansible-lint config as a workaround.

@cidrblock WDYT?

@ssbarnea ssbarnea added the feedback-needed Divergent oppinions, additional feedback is desired in order to unblock it. label Jun 28, 2022
@netsandbox netsandbox changed the title add support for yaml extension in DEFAULT_KINDS add support for yaml extension in DEFAULT_KINDS for Ansible configs where still missing Jun 28, 2022
@netsandbox
Copy link
Contributor Author

The title is a little bit misleading because the tool does already recognize .yaml ...

I have updated the title.

I am not sure we want to allow this because using these extensions will likely break LOTS of tools in the ecosystem, as most of them only recognize one filename.

We are talking here about Ansible and Ansible always support both, .yml and .yaml extension.
And if Ansible support both extension, ansible-lint also have to.

Sadly Ansible still defaults to the .yml extension, while YAML themself recommends using the .yaml extension:
https://yaml.org/faq.html

Also while Ansible support both extensions, sadly there is no mention of this in the documentation.

Copy link
Contributor

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I hate how ansible defaults to .yml in so many places. But, at least for galaxy.yml, that's required.

src/ansiblelint/config.py Outdated Show resolved Hide resolved
@netsandbox
Copy link
Contributor Author

I also reverted .config/ansible-lint.yml which also doesn't support the .yaml extension 😢 :

project_filenames = [".ansible-lint", ".config/ansible-lint.yml"]

So there are only inventory and requirements files left.

@cognifloyd
Copy link
Contributor

Both inventory and requirements files are entirely user defined. Ansible only uses them if someone explicitly listed them in a config file, env var, or cli arg. So, we should support .yaml at least for those.

I require my team to use .yaml for inventory files and anything else that is allowed to avoid the legacy .yml extension.

@ssbarnea
Copy link
Member

I do have some doubts that you will be able to upload collections to galaxy using a requirements.yaml file instead of requirements.yml. Someone needs to try it.

@ssbarnea ssbarnea added the bug label Jun 28, 2022
@ssbarnea ssbarnea changed the title add support for yaml extension in DEFAULT_KINDS for Ansible configs where still missing Allow yaml extension for inventory and requirements Jul 11, 2022
@ssbarnea ssbarnea removed the feedback-needed Divergent oppinions, additional feedback is desired in order to unblock it. label Jul 11, 2022
@ssbarnea ssbarnea merged commit 6bf6371 into ansible:main Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants