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

Proposal for environment context #148

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

hfjn
Copy link
Contributor

@hfjn hfjn commented May 12, 2020

This is a proposal for a new environment context which takes precedence of the config file.
We had the need for a context which can be configured for automation without touching files.
Happy to hear your feedback.

@hfjn hfjn added the enhancement New feature or request label May 12, 2020
@swenzel
Copy link
Contributor

swenzel commented May 12, 2020

Hmm instead of going through all that trouble of creating and mapping config variables, you could also create an environment variable ESQUE_CONFIG_YAML that can hold the whole esque config as yaml string. That would be more dynamic, future proof and would only require minimal changes.

@hfjn
Copy link
Contributor Author

hfjn commented May 13, 2020

Hm. I get your point, in general. But that would only get rid of the need to write a file, not to create a YAML in general. :D

@swenzel
Copy link
Contributor

swenzel commented May 14, 2020

You wrote:

without touching files.

So I thought, that was your problem 😄

Well, although I don't see how setting 15 environment variables is easier than creating a yaml string, I'm always a fan of "letting the user choose". So I'm not entirely against it.
I'm just afraid that we might have to rename, add and/or remove some of the variables while our config evolves. We do have a migration mechanism for file based configs but not for environment variables. Do you think a separate command to add a section to the config would help you?
Something like esque config add-context foo --bootstrap-servers broker1,broker2 --schema-registry registry ...
I think it's easier to keep a command stable than it is to keep the environment variables the same over time. But that's open for discussion 🙂

@hfjn
Copy link
Contributor Author

hfjn commented May 14, 2020

Guess I should have been clearer. Sorry for that. :D
I think you're right with your approach of not wanting to make it unnecessarily complicated and I think the idea of just adding another command is good. I'll see with what I can come up with over the weekend. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants