-
-
Notifications
You must be signed in to change notification settings - Fork 47
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(example): support custom string in redact_sensitive script #90
base: master
Are you sure you want to change the base?
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.
❌ Changes requested. Reviewed everything up to 94e0309 in 1 minute and 14 seconds
More details
- Looked at
63
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. examples/redact_sensitive.py:124
- Draft comment:
The updated '_redact_event' function signature correctly takes the replacement string parameter. Ensure that similar naming conventions are used consistently across the script. - Reason this comment was not posted:
Comment did not seem useful.
2. examples/redact_sensitive.py:23
- Draft comment:
Removal of the hardcoded REDACTED constant: consider renaming this user-supplied replacement string (now passed as a parameter) to a more descriptive, lower-case name (e.g., replacement_str) to avoid confusion with constants. - Reason this comment was not posted:
Marked as duplicate.
3. examples/redact_sensitive.py:60
- Draft comment:
User input for the replacement string is now added. Consider validating that the input is not empty to avoid accidental redaction with an empty string. - Reason this comment was not posted:
Marked as duplicate.
4. examples/redact_sensitive.py:124
- Draft comment:
The function signatures for _redact_bucket and _redact_event now take a 'REDACTED' parameter. Consider renaming the parameter to a lower-case, more descriptive name (e.g., replacement_str) since it is a runtime value, not a constant. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_auQBnfiCSYraGj0N
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
examples/redact_sensitive.py
Outdated
@@ -59,6 +57,8 @@ def main(): | |||
input("Enter a regex indicating sensitive content: ").lower() | |||
) | |||
|
|||
REDACTED = input("Enter the string used to replace sensitive content: ") |
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.
This is too long but I don't know how to cut it down. Now it should be even longer due to the empty fallback.
fallback to "REDACTED" if input is empty
Just like the title said. I thought I open a PR months ago but obviously I didn't.
Would fallback to previous "REDACTED" if the input is empty.
Command line example (ignore the make command):
By the way, would you accept a PR to add Inline script metadata per PEP 723 in these scripts?
This is useful because I "install" activitywatch via Linux tarball and do not want to
pip install aw-client
again to use these scripts. With inline script metadata, I can run them simply byuv run redact_sensitive.py
and it would handle dependency/venv automatically.It would look like this:
Important
Adds support for custom replacement strings in
examples/redact_sensitive.py
for redacting sensitive data.main()
for custom replacement string for sensitive content._redact_bucket()
and_redact_event()
for redaction._redact_bucket()
to accept and use custom replacement string._redact_event()
to replace sensitive content with custom string.REDACTED
string from global scope.This description was created by
for 94e0309. It will automatically update as commits are pushed.