Skip to content
This repository has been archived by the owner on Nov 11, 2019. It is now read-only.

Use YAML as default config file format #47

Closed
wants to merge 1 commit into from
Closed

Use YAML as default config file format #47

wants to merge 1 commit into from

Conversation

AvdN
Copy link

@AvdN AvdN commented Sep 5, 2015

Further abstraction from ConfigParser, introduction of YAML as
(default) config file format.

  • introduces NoOptionError, and have IniConfig.get() throw that and
    remove further cmdline.py and util.py dependency on ConfigParser
  • steve.yaml/YamlConfig is now the default name/type for config file
  • old projects (steve.ini) are still found and will be read
  • parametrized test for config file creation/retrieval (on util.py level)
    for both config types

Further abstraction from ConfigParser, introduction of YAML as
(default) config file format.

- introduces NoOptionError, and have IniConfig.get() throw that and
  remove further cmdline.py and util.py dependency on ConfigParser
- steve.yaml/YamlConfig is now the default name/type for config file
- old projects (steve.ini) are still found and will be read
- parametrized test for config file creation/retrieval (on util.py level)
  for both config types
return self._project_path

@property
def project_file(self):
Copy link
Member

Choose a reason for hiding this comment

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

What's a project file? Is that the config file or any file in the project?

Copy link
Author

Choose a reason for hiding this comment

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

steve.ini resp. steve.yaml depending on the type of project file the project started with (ie. this supports any already created project without having to do a conversion).

@willkg
Copy link
Member

willkg commented Sep 5, 2015

I think that's everything. I'm a little bummed this is adding a lot of additional code just to add support for yaml config files.

I'm wondering whether instead of supporting all the possible things, it's better to build an "updateproject" command that looks a the status of the project and applies migrations to it. So then instead of supporting ini and yaml config files, we'd just support yaml files and the updateproject command would migrate ini files to yaml files. That seems like a more viable approach that covers other things we're changing, too.

@willkg
Copy link
Member

willkg commented Sep 5, 2015

@codersquid ^^^ What do you think about that? Do you value backwards compatibility for older projects? Is having an "updateproject" command acceptable?

@willkg
Copy link
Member

willkg commented Sep 5, 2015

@codersquid Also, this PR is pretty slick. It adds a bunch of things that'll make config files easier to manage plus it starts the work for a status file.

@AvdN
Copy link
Author

AvdN commented Sep 5, 2015

That it is a lot of code is because of the backwards compatibility. As I am still vague about how the tool is used in real life, I would loath to break things and have people being blocked by functionality that I removed. AFAIK you can take an old (read steve.ini based) project and run this code without problems. You should even be able to use the next PR that I have prepared and get YAML support, while using such "old" projects transparently. It is of course more easy to pull out the old engine and put in a new in its place, but I don't think that is the only correct way foreward. And that would indeed require some conversion routine, which I, going this route of backwards compatbility, was not thinking of providing.

@codersquid
Copy link
Contributor

I don't need backwards compatibility. I've never gone back to update pyvideo records from a steve project. There have only been 1 or 2 times I wanted to do that. It's really rare. Please don't feel obligated to make things backward compatible.

@codersquid
Copy link
Contributor

Thanks very much for all this work! I can try this out this weekend.

@willkg
Copy link
Member

willkg commented Sep 5, 2015

@AvdN I think there's some misunderstandings about the current state of steve. The only users of steve that we need to support are me, @codersquid and you (assuming you decide to help out adding additional conferences). That's it.

If you have questions about usability, you should ask @codersquid and I since we're the users of steve since I started throwing it together. Probably better not to assume.

One of the things I don't want to do is add a lot of maintenance burden unless there are really good reasons for doing that. Large maintenance burdens lead to fast bitrot. This is a hobby project, so I'd rather have less code and less to maintain than fulfill all possible usage scenarios.

I think given that and the fact that @codersquid doesn't particularly care about backwards compatibility, I'd like to nix all the .ini stuff and just go with .yaml for config files. That should hopefully reduce a lot of the code here that adds the abstraction layer for configuration formats.

Also, one thing to note is that you're tackling issues that aren't fleshed out. There's a lot unspecified. In the future, it might make more sense to flesh out the bugs before implementing.

@AvdN
Copy link
Author

AvdN commented Sep 6, 2015

@codersquid I hope that with the next PR changes, this is actually useful. The config changes from .ini to .yaml only make sense (IMO) if the metadata is primarily handled in YAML as well, so there is essentially only one format for which you need to remember the quirks ( such as are quotes needed around strings, do you use ":" or "=" to separate keys and values. )

@willkg
Copy link
Member

willkg commented Sep 6, 2015

I think #48 obsoletes this PR, so I'm closing it out.

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

Successfully merging this pull request may close these issues.

None yet

3 participants