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

feat: use host/user/pass from env vars #406 #408

Merged
merged 2 commits into from
Oct 17, 2024
Merged

Conversation

mahnunchik
Copy link
Contributor

Feature description: #406

  • It is not a breaking change
  • Config parsing kept as is
  • Config validation kept as is
  • Environment variables has precedence over config

I think it is simplest way to bring this feature to mqtt-io

@mahnunchik
Copy link
Contributor Author

@flyte @mschlenstedt @BenjiU may I ask for the review of this PR?

@BenjiU
Copy link
Collaborator

BenjiU commented Oct 17, 2024

ah damn! I typed this last week, but didn't sent it :-/

That looks nice! One improvement would be, to just have the PW via ENV and the other configs from the file, but that can be improved later.

Would you please document this configuration (in the readme)?

@mahnunchik
Copy link
Contributor Author

One improvement would be, to just have the PW via ENV and the other configs from the file, but that can be improved later.

All variables have default from the config file, so it is possible to specify only necessary env vars to override specific config variables.

My goal was to be able to specify all the variables ​as env vars ​that the mosquito writes to the automatic discovery "registry" https://github.com/home-assistant/addons/blob/d5f529465229c3075ae3306769e1378d9b507028/mosquitto/rootfs/etc/services.d/mosquitto/discovery#L18-L50

@mahnunchik
Copy link
Contributor Author

@BenjiU I've opened another PR about readme updates haha 😆 #405

But I can document it manually

@BenjiU
Copy link
Collaborator

BenjiU commented Oct 17, 2024

Ah, your right, of course! Perfect!

yeah, just a short manual documentation, so someone knows it is there. then i will merge and release it.

Thanks for your contribution and i'll have a look at #405

@mahnunchik
Copy link
Contributor Author

@BenjiU documentation has been added

@BenjiU BenjiU merged commit 0d380db into flyte:develop Oct 17, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants