-
Notifications
You must be signed in to change notification settings - Fork 1
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(config): Env overrides for all standard config values #15
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'm going to hold off on merging until I get an automated release mechanism in place, should be by EOW 🙂
@forslund Looks like it's not compiling as-is? https://github.com/OscillateLabsLLC/ovos-rust-messagebus/actions/runs/11999520451/job/33578793820?pr=15 |
I've fixed the clippy issues but got trouble with the tests running in parallell despite the "[serial]" attribute. (this worked last week!) so I also updated the CI-config to only use a single thread. |
Thanks for taking care of that. I got the automated release workflow in place but unfortunately it created a conflict. If you wouldn't mind resolving that I'll go ahead and merge this in and we'll release 1.1 |
New environment variables: - `OVOS_BUS_ROUTE` (default: `/core`) - `OVOS_BUS_USE_SSL` (default: `false`) NOTE: If the environment variable exists SSL will be enabled.
d645815
to
4f79b88
Compare
The tests seem to have been completely removed. I've added them back as a separate github action yaml |
This adds env overrides for the missing config options (excluding "extras") and may possibly resolve #12.
To note: OVOS_BUS_USE_SSL will set the ssl-flag if the environment variable exists. The parsing of bools seemed a bit limited using the standard
.parse()
so instead of requiring it to be EXACTLYtrue
this felt like a more safe way.