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

Log to syslog #55

Merged
merged 7 commits into from
Feb 1, 2022
Merged

Log to syslog #55

merged 7 commits into from
Feb 1, 2022

Conversation

hugoh
Copy link
Contributor

@hugoh hugoh commented Jan 31, 2022

Unifies logging to use logging and adds option to log to syslog.

Note that it works on Linux, and I cannot figure out why it doesn't on macOS (/var/run/syslog should be the socket to write to…).

Implements #54 on Linux, maybe on macOS too.

Moved all the logging from print_and_log to logging, but syslog doesn't
work. Need to debug.
@highvolt-dev
Copy link
Owner

Thanks! Looks like I noticed two things:

  1. The log format changed so the log level is no longer in square brackets. This affects parsing within delta logging as it is not updated to remove the brackets in its pattern declaration.
  2. This PR introduces f strings which bumps up the python version requirement to be v3.6
  3. I can't test against OS X either, but seems like we should give a runtime error and exit code 2 if syslog logging is enabled on a Windows platform unless we have it attempt to register and use the windows application event log with a distinct event source.

@hugoh
Copy link
Contributor Author

hugoh commented Feb 1, 2022

I addressed all comments. Not sure how I ended up with a different log format to start with…

@highvolt-dev
Copy link
Owner

Thanks for the quick turnaround. Looks like the syslog_socket variable is referenced outdented from the block it is defined in the loop now.

Looks like we would want to adjust that reference, but beyond that, ready to merge your PR since we are in the middle of beta testing v2.0 and don't need this to be perfect

@hugoh
Copy link
Contributor Author

hugoh commented Feb 1, 2022

Should be fixed. Thanks.

@highvolt-dev
Copy link
Owner

Maybe I'm tired, but looks like this would be missing whitespace before 'via'? Sorry for all the revisions!

@hugoh
Copy link
Contributor Author

hugoh commented Feb 1, 2022

Oopsie. Fixed.

@highvolt-dev
Copy link
Owner

Thanks. The README is not up to date but I'll merge now! I can check out windows logging if I get the chance and document the system logging at that point perhaps? Thanks for your ongoing contributions!

@highvolt-dev highvolt-dev merged commit 0174946 into highvolt-dev:main Feb 1, 2022
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.

None yet

2 participants