#38 Allow to specify default args in a config file
Closed: Fixed 9 months ago by lruzicka. Opened a year ago by kparal.

Some default args are quite long, like --skip-bodhi-comments, and instead of requiring users to write it every time when invoking FEK, it would be nice if they could specify default args in a config file.


Example: youtube-dl simply reads the config file add then passes those options to the command line parser:
https://github.com/ytdl-org/youtube-dl/blob/master/README.md#configuration

Metadata Update from @lruzicka:
- Issue assigned to lruzicka

a year ago

Sure, this a good idea. I will work on it.

Metadata Update from @lruzicka:
- Custom field story_points adjusted to 2

a year ago

Metadata Update from @lruzicka:
- Custom field story_points adjusted to 3 (was: 2)

a year ago

I realized that the logic of this would have to be a little more complex than I expected, so I increase the complexity a little bit as it took me some time to figure out what I really want.

The documentation for the config file was provided in a git commit, but nowhere user-accessible. I added it to:
https://fedoraproject.org/wiki/Fedora_Easy_Karma#Using_a_config_file

Metadata Update from @kparal:
- Issue close_status updated to: None (was: Fixed)

10 months ago

Issue close_status updated to: None (was: Fixed)

Pagure... 🤷

I created the config file according to the documentation, and it crashes for me. (Also, it's not clear whether it should be one arg per line, or all args on a single line, that should be clarified. But I tested just one arg, and it still crashes):

$ cat ~/.config/fedora-easy-karma/config.ini 
pages
$ python fedora-easy-karma.py --config-file
Traceback (most recent call last):
  File "/home/kparal/devel/qa/fedora-easy-karma/fedora-easy-karma.py", line 1094, in <module>
    fek = FedoraEasyKarma()
  File "/home/kparal/devel/qa/fedora-easy-karma/fedora-easy-karma.py", line 654, in __init__
    self.cfg.read_configuration_file(self.options.config_file)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kparal/devel/qa/fedora-easy-karma/fedora-easy-karma.py", line 419, in read_configuration_file
    file_configuration.read(path)
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/usr/lib64/python3.13/configparser.py", line 735, in read
    self._read(fp, filename)
    ~~~~~~~~~~^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/configparser.py", line 1050, in _read
    ParsingError._raise_all(self._read_inner(fp, fpname))
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/configparser.py", line 334, in _raise_all
    raise next(exceptions).combine(exceptions)
configparser.ParsingError: Source contains parsing errors: '/home/kparal/.config/fedora-easy-karma/config.ini'
    [line  1]: 'pages\n'

If the config file is incorrect, we should clarify documentation, and also just print an warning/error with some hints (a link to docs) and possibly continue, instead of crashing.

Metadata Update from @kparal:
- Issue status updated to: Open (was: Closed)

10 months ago

Also, I'm really sorry that I haven't been able to provide feedback before, and I couldn't test the current state due to crashes, but the current implementation (as I read the docs) makes me unhappy. I wanted to have a config file to save on typing, i.e. so that I don't need to type --pages every single time. But instead, I have to type --config-file every single time, which is even longer.

Also, the current config file logic seems reversed to all other programs I can think of. Ordinarily, the config file specifies the defaults, it's auto-loaded, and it has the lowest priority. And you can override it with a cli argument, which has the highest priority. That's the rule, everywhere where I can remember. But the current implementation is the very opposite. It will confuse users, and I don't understand the point of this. (Sorry for being a pain).

The config file is currently in INI format and is parsed using a standard INI parser. This means the format must follow INI rules — e.g., pages = 1, with each option on its own line.

I'm still undecided on the best approach for handling where the settings should come from. Ideally, I’d like to load defaults from a config file, which can then be overridden via the command line. However, if we allow specifying an additional config file via CLI, it introduces ambiguity: which values should take precedence?

For now, I’ve resolved this by only using a config file if it is explicitly specified:

If you use --config-file without a path, a default config is loaded.

If you use --config-file <path-to-file>, a specific config is loaded instead (since you mentioned wanting multiple configs).

This behavior is not yet documented, and since it hasn't been released yet, now is a good time to refine or redesign the logic if needed.

Note - I have a proof-of-concept of a simpler config file parsing, but I won't be able to publish it today, so hopefully early next week.

An alternative approach is in #52

Please note that I'm not married to either approach. I created #52 to showcase that it can be implemented quite simply (and also I enjoyed the exercise). But if you prefer the "full functionality" approach, I'm fine with it as well. It's really up to you which approach you like more.

Lukas decided to swap the existing approach (15eacf7 + 759324f) with the approach from #52. I'll update the PR with commits that will revert the last two mentioned commits and use the approach from #52 instead.

Sure, go ahead. Thanks.

Commit c8f38d6 relates to this ticket

This issue has been migrated to Fedora Forge:
https://forge.fedoraproject.org/quality/fedora-easy-karma/issues/38

Please continue any further discussion there.

Log in to comment on this ticket.

Metadata
Related Pull Requests
  • #51 Merged 9 months ago
  • #47 Merged 10 months ago