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

Set config file path via PFTQCONFIG env var #19

Merged
merged 2 commits into from
Aug 6, 2020

Conversation

ordovicia
Copy link
Contributor

With this PR merged, pftaskqueue will read PFTQ_CONFIG environment variable (if set) to get the path of a config file.
--config flag is prioritized over PFTQ_CONFIG env var, so this change should not break the current behavior.

This feature would be useful when one does not want to place .pftaskqueue.yaml on a home directory and does not also want to type --config=... flag every time.

Copy link
Contributor

@everpeace everpeace left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

Currently, pftskqueue support config values from envvars. This feature can load foo.bar value from PFTQ_FOO_BAR. Thus, supporting PFTQ_CONFIG for config filename might cause a conflict in the future although current configuration doesn't have config key.

If we can find different prefix, I'm ok. What do you think?

@ordovicia
Copy link
Contributor Author

Thank you for your quick response.

I came up with three options:

  1. PFTQCONFIG
  2. PFTASKQ_CONFIG
  3. PFTASKQUEUE_CONFIG

I am a little inclined to the option 1 because it is the most concise.
c698823 changes the environment variable name to PFTQCONFIG.

But I am fine with other names.
What name do you think is the best?

@ordovicia ordovicia changed the title Set config file path via PFTQ_CONFIG env var Set config file path via PFTQCONFIG env var Aug 6, 2020
Copy link
Contributor

@everpeace everpeace left a comment

Choose a reason for hiding this comment

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

I came up with three options:

  • PFTQCONFIG
  • PFTASKQ_CONFIG
  • PFTASKQUEUE_CONFIG

I am a little inclined to the option 1 because it is the most concise.

Agreed. I also prefer to PFTQCONFIG. Thanks!!

@everpeace everpeace merged commit 926adbd into pfnet-research:master Aug 6, 2020
@everpeace everpeace added the release-note/feature Feature category in release note label Aug 6, 2020
@ordovicia ordovicia deleted the config-path-from-env-var branch August 7, 2020 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/feature Feature category in release note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants