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

State file to track autoscan #36

Merged
merged 8 commits into from
Oct 12, 2020
Merged

State file to track autoscan #36

merged 8 commits into from
Oct 12, 2020

Conversation

mpuckett159
Copy link
Contributor

closes #20
Added code to create a state file to track the liveness of the autoscan feature across application restarts.

run.py:
Add logging to console and listening group to denote the loaded state of the autoscanner

env.py:
Update the _State class to check for the provided state file on load and set the LISTENING property appropriately.

Add update_listening_status method for the _State class to change the property and update the existence of the file as required.

Add get_listening_status_notice method for the _State class to provide an easy way to return the notification message to be sent to the Signal listener group.

messages.py:
Update code to make use of the new methods of the _State class.

Added code to create a state file to track the liveness of the autoscan feature across application restarts.

run.py:
Add logging to console and listening group to denote the loaded state of the autoscanner

env.py:
Update the _State class to check for the provided state file on load and set the LISTENING property appropriately.

Add update_listening_status method for the _State class to change the property and update the existence of the file as required.

Add get_listening_status_notice method for the _State class to provide an easy way to return the notification message to be sent to the Signal listener group.

messages.py:
Update code to make use of the new methods of the _State class.
Copy link
Collaborator

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really great start. I have a personal preference for using the pathlib standard library over os.path and affiliates because I find it simplifies the logic. Let me know what you think!

signal_scanner_bot/bin/run.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Outdated Show resolved Hide resolved
mpuckett159 and others added 5 commits October 11, 2020 16:14
Remove Signal group alert on service start to notify users/members of the current autoscan status.

Co-authored-by: Madison Bowden <[email protected]>
pathlib is simpler to work with than os.path so we're swapping over to that. This commit is to migrate functionality to this library.

Co-authored-by: Madison Bowden <[email protected]>
.env.example:
Not exactly sure what happened here but fixed the incorrect env vars

run.py:
Remove unused signal library import

env.py:
Change to pathlib from os.path
Add default state file Path object for _State class
Rework update_listening_status to allow for a str output type. Made use of new pathlib library.
Add import of optional AUTOSCAN_STATE_FILE_PATH variable and change the STATE variable declaration to make use of new _State class properties.

messages.py:
Also not sure what happened here but fixed incorrect variable names.
messages.py:
Removed START_LISTENING and STOP_LISTENING constants from file, migrated to env.py file with the autoscan functionality updates.

env.py:
Added START_LISTENING and STOP_LISTENING constants from messages.py file. Added new constants START_LISTENING_NOTIFICATION and STOP_LISTENING_NOTIFICATION to store the notification messages sent to the Signal group.
Remove input default for _State class deceleration and moved this to the _env functionality for consistency.
Updated the status check logic to make use of the new constants as well as the function returns.
Removed the `()`'s from around the status in the logging message not sure why I did that in the first place.
Updated the _env file to handle an issue with how the defaults work now that we're casting everything. Previously the default wouldn't be cast properly and could potentially (though unlikely) get messed up. This makes the default parameter for the _env function a little nicer now. OS environment variables only come down as strings so this is safe from a typing perspective as well.
Add _cast_to_path function to take a string and return a pathlib Path object.
Update AUTOSCAN_STATE_FILE_PATH declaration to set the default path to our desired default path, and add the new casting function to convert the environment variable into a pathlib Path object.
Finally, update the STATE initialization line to make use of all the above changes and simplify further.
Copy link
Collaborator

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple more thoughts, but looks great as-is. Nice work!

signal_scanner_bot/env.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Outdated Show resolved Hide resolved
signal_scanner_bot/env.py Show resolved Hide resolved
signal_scanner_bot/messages.py Outdated Show resolved Hide resolved
env.py:
Removing Optional type from import as it is no longer used in the file 🎉
Updating error message to be more modular.
Simplify get_listening_status_notice return logic and I guess this is how black wants to format it so that's what we're going with.

messages.py:
removing unnecessary type conversion now that update_listening_status only passes a str type back.

.env.example:
Add some more explanation text to the AUTOSCAN_STATE_FILE_PATH. Hope to eventually get some good sanity checks in the type casting function to verify that any paths provided aside from the default make sense and are reachable and persistent.
Copy link
Collaborator

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo! 🎉

@mpuckett159 mpuckett159 merged commit f6eaae3 into master Oct 12, 2020
@mpuckett159 mpuckett159 deleted the feature/state-file branch October 12, 2020 05:32
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.

Enable flag should be written to a file so that it persists over restarts
2 participants