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 .yml as well as .yaml #67

Closed
mitom opened this issue Mar 1, 2017 · 12 comments
Closed

Allow .yml as well as .yaml #67

mitom opened this issue Mar 1, 2017 · 12 comments

Comments

@mitom
Copy link

mitom commented Mar 1, 2017

Could you allow using .yml as well as .yaml for the configuration files? I'd say they are equally common and they are both valid. .yml files are simply ignored at the moment.

@jamesroutley
Copy link
Contributor

Hey @mitom we've decided to not support .yml extensions. We currently use the filesystem to guarantee name uniqueness, and we can't do that if we support .yml and .yaml.

Imagine you have:

.
└── config
    └── dev
        ├── vpc.yaml
        └── vpc.yml

And you run sceptre launch-stack dev vpc we'd have no way of telling which one you wanted.

We went with .yaml because it's the official extension.

@mitom
Copy link
Author

mitom commented Mar 1, 2017

It being the official extension is a good point, I wan't aware of there being a preference! However, it's so commonly interchanged (https://github.com/cloudreach/sceptre/blob/master/circle.yml 😛 ) I would expect it to work as well. Alternatively, a friendlier error for not finding a stack:

sceptre create-stack dev common
Traceback (most recent call last):
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/bin/sceptre", line 11, in <module>
    sys.exit(cli())
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/lib/python2.7/site-packages/click/core.py", line 716, in __call__
    return self.main(*args, **kwargs)
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/lib/python2.7/site-packages/click/core.py", line 696, in main
    rv = self.invoke(ctx)
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/lib/python2.7/site-packages/click/core.py", line 889, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/lib/python2.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/lib/python2.7/site-packages/sceptre/cli.py", line 93, in decorated
    return func(*args, **kwargs)
  File "/Users/tamasmillian/.pyenv/versions/2.7.12/lib/python2.7/site-packages/sceptre/cli.py", line 243, in create_stack
    response = env.stacks[stack].create()
KeyError: u'common'

(the error above is due to me naming the config common.yml instead of common.yaml)

As for uniqueness, It seems easy to validate uniqueness based on the filename - ignoring the extension - and throwing an error or falling back to the yaml as a the preferred one due to it being the official extension?

@theseanything
Copy link
Contributor

I'd say if this did get implemented, I'd lean towards throwing duplicate stack config exception. To be explicit and prevent any confusion over which file was being used.

@jamesroutley
Copy link
Contributor

I think the thing to do for now improve the error thrown when .yml is used. We can search config/ for .yml files and throw an error warning the user that the .yaml extension should be used.

Allowing either and throwing an error if a duplicate is found could lead to issues. If two projects were happily using .yml and .yaml respectively, and wanted to share code, the difference in extensions could lead to issues avoided if we push everyone to use the same extension.

Also, I think think this issue may become redundant if we implement #62 in v2.

@theseanything
Copy link
Contributor

theseanything commented Mar 1, 2017

@jamesroutley could you elaborate on the error you could see happening when code is shared?

Tbh, another reason this hadn't been done, was it was low priority and we were trying to keep maintainability - Implementing throwing an exception to warn .yml being used kinda negates this.

@jamesroutley
Copy link
Contributor

so if you have any .yml file under config/ (i.e. config/**.yml), we throw e.g. an UnsupportedTemplateFileTypeError which says File x.yml has an unsupported file extension. Please use .yaml

@galbinasu and I looked at the implementation the first time this issue was raised (on BitBucket) and we decided that it wasn't worth adding support for the uniqueness reason mentioned in my first post. If we only have one extension, we defer uniqueness checking to the filesystem. If we have both, we have to implement it ourselves.

@theseanything
Copy link
Contributor

theseanything commented Mar 1, 2017

@jamesroutley sorry I meant what issues do you see happening when code is shared, like you mentioned. (Not the error about unsupported file extension). Not sure I see the disadvantages...

Right - but if we are going to have implement something either way - if we throw a UnsupportedTemplateFileTypeError or we throw a DuplicateConfigurationFileError. Amount of code/logic probably is about the same.

@mitom
Copy link
Author

mitom commented Mar 1, 2017

@jamesroutley #62 would help some but definitely wouldn't solve it. The --recursive option will error on missing stacks - or not do anything, I am not sure - for .yml files. The autocomplete would provide visibility, but it will happily complete to path/to/stack.yml as implying it's a valid input.

I don't see how shared code or collaboration would cause errors with this, in case 2 projects are merged (or partially merged) there are definitely more work to be done than renaming 1 file and changing the resolver for some parameters to avoid the duplicate.

I understand the aim for simplicity, but I still disagree on behalf of pretty much everything else supporting .yml as an interchangeable extension for .yaml.

@jamesroutley
Copy link
Contributor

@seanrankine sounds good - let's go with that. So when we search for stacks we'll look for (.yml|.yaml) and throw an error if a directory contains a .yml and .yaml with the same basename?

@theseanything
Copy link
Contributor

Yup!

@JuanCanham
Copy link

I know this is an old thread, but can we close as will not implement?

.yaml is the "correct" extension as per http://yaml.org/faq.html and most of the workarounds are quite hacky

@ngfgrant
Copy link
Contributor

ngfgrant commented Jul 4, 2018

Yeh I agree @JuanCanham. It is not a priority at the moment, I will make sure it is clear in the docs to use .yaml and not .yml.

@ngfgrant ngfgrant closed this as completed Jul 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants