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

fix(examples): fixed redact_sensitive.py #61

Merged
merged 3 commits into from Apr 26, 2022

Conversation

yumemio
Copy link
Contributor

@yumemio yumemio commented Dec 16, 2021

This PR is a fix for redact_sensitive.py, which is a program to search and delete events by its name.

The script has two problems:

  1. Redacted events are left intact and visible, because only insertion of events with the name "REDACTED" is performed.
  2. The program prematurely exits, leaving some buckets not queried.

This PR fixes the 1st issue by creating a Python API for DELETE requests (in client.py), and calling it from redact_sensitive.py. I also implemented a unit test for the relevant function.

Fixing the second issue is trivial, and is addressed at L70 of redact_sensitive.py.

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Dec 16, 2021

This pull request introduces 1 alert when merging 9f7e666 into 6a842d3 - view on LGTM.com

new alerts:

  • 1 for Testing equality to None

@yumemio
Copy link
Contributor Author

yumemio commented Dec 18, 2021

This pull request introduces 1 alert when merging 9f7e666 into 6a842d3 - view on LGTM.com

new alerts:

* 1 for Testing equality to None

Okay, I fixed the lint issue.

@ErikBjare
Copy link
Member

ErikBjare commented Mar 3, 2022

Redacted events are left intact and visible, because only insertion of events with the name "REDACTED" is performed.

This should not be the case. When an insert_events is made given an event with an ID, the new event should replace an existing event with the same ID. If events are not being replaced, that's a bug somewhere.

The program prematurely exits, leaving some buckets not queried.

Nice catch!

This PR fixes the 1st issue by creating a Python API for DELETE requests (in client.py), and calling it from redact_sensitive.py. I also implemented a unit test for the relevant function.

I just added the same endpoint in this commit: 32c25fd

@lgtm-com
Copy link
Contributor

lgtm-com bot commented Apr 24, 2022

This pull request introduces 1 alert when merging 592af3b into 90f3b69 - view on LGTM.com

new alerts:

  • 1 for Variable defined multiple times

@ErikBjare ErikBjare merged commit f6272c9 into ActivityWatch:master Apr 26, 2022
@ErikBjare
Copy link
Member

Merged. Thanks for contributing @yumemio!

@ErikBjare
Copy link
Member

ErikBjare commented Apr 27, 2022

Should have taken a closer look at this before merging... Looks like we ended up with duplicate function definitions, and incompatible function signatures.

@yumemio
Copy link
Contributor Author

yumemio commented Apr 28, 2022

@ErikBjare Sorry, I did a premature push to my repo. Working on a fix now.

EDIT: I noticed your commit df8776c fixed the issue. Sorry for the mess...

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