Fix daemon and python 3.7 issues v2 #151
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The indentation was wrong, causing the daemon to only process the last block of outout, which would only show up with a lot of output.
We can no longer have python automatically convert binary to/from string data, so be explicit about it. Also, the TargetCLI __del__ method was testing for a non-existant attribute, so close our socket if open.
Add Documentation entries, cleaned up socket activation entries.
There's no need to return a zero-length string. Before being fixed, this caused a hang when shutting down.
Save the error exit for unknown exits. This makes systemd happier, since it stops us with a signal.
This allows the main loop to get an exception in its wait loop when a signal arrives, rather than block forever. Before this, topping the daemon required sending a KILL signal or sending one last command, so that the loop could check the "received a signal" flag.
Regular socket communications are fallen back to when no systemd socket is present.
Before, we would get a "Bad File Descriptor" message when a signal causes closure of the socket while waiting on a socket message.
pkalever
approved these changes
Oct 11, 2019
There was a problem hiding this comment.
Leaving very minor findings below.
Tested it on fedora 30, it works well for me.
Overall the PR looks good to me.
Thanks @gonzoleeman
|
Would you like me to fix the spelling errors in an additional commit? |
This is not important, for projects which I maintain, I generally fix it while merging, avoiding the need to respin for the author/contributor :-) Or IMHO, we can fix it later when we come across that part of code in our future PR's. Thanks! |
|
Looks good to me, thanks @gonzoleeman |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I believe this pull request fixes all issues raised in the first version: