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

Bug merging configuration across .env, command line, and defaults #31

Closed
danhausman opened this issue Jan 1, 2022 · 7 comments
Closed
Labels
bug Something isn't working

Comments

@danhausman
Copy link

I grabbed the latest version and the logging is not working. Where it was on 12/30. Also the print configuration on the latest version is not working.

@danhausman
Copy link
Author

I have this in my config file:

tmo_print_config=True
tmo_logfile=/home/pi/tmo-monitor/tmo.log
tmo_log_all=True

It is creating a tmo-monitor.log too which is just empty. It is not writing there. I tried removing the tmo_logfile setting and it still does not wiret to the tmo-monitor.log

@highvolt-dev
Copy link
Owner

I am able to reproduce this problem, thank you for reporting @danhausman

@highvolt-dev
Copy link
Owner

highvolt-dev commented Jan 1, 2022

@danhausman my config started to be parsed correctly by adding quotes to the path you assigned to tmo_logfile

edit: it still does not work correctly though, I think that it's getting overridden by the new logging flags
edit 2: yes, it is being caused by the command line flag parsing

@highvolt-dev
Copy link
Owner

There are multiple bugs relating to configuration from .env vs the command line flags and defaults. Currently working on fixes.

@highvolt-dev highvolt-dev added the bug Something isn't working label Jan 1, 2022
@highvolt-dev highvolt-dev changed the title Logging issues with latest version Bug merging configuration across .env, command line, and defaults Jan 1, 2022
highvolt-dev added a commit that referenced this issue Jan 1, 2022
@highvolt-dev
Copy link
Owner

@danhausman the issues should be fixed now - I'll keep this issue open until you have a chance to confirm!

@danhausman
Copy link
Author

@highvolt-dev confirmed new version working as expected. Thank you so much, I appreciate the work you are doing here.

@AndrewPardoe
Copy link
Contributor

Thanks for catching this @danhausman and thanks for the fixes, @highvolt-dev. These many-line changes are always hard to test completely. Sorry for the bugs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants