-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add script to delete empty history entries from LOS, Links, and Devices #815
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #815 +/- ##
==========================================
- Coverage 94.88% 94.87% -0.02%
==========================================
Files 91 92 +1
Lines 3892 3939 +47
==========================================
+ Hits 3693 3737 +44
- Misses 199 202 +3 ☔ View full report in Codecov by Sentry. |
I have reservations about using this, especially in production, but I think it would be better to have this and not need it than need this and not have it. |
self.deduplicate_history(Link.objects.all()) | ||
self.deduplicate_history(Device.objects.all()) | ||
self.deduplicate_history(LOS.objects.all()) |
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.
Nit: maybe the models should be a parameter
history_model = m.history.model | ||
table_name = history_model._meta.db_table | ||
|
||
# Delete history from each object. Atomic block makes it go faster |
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.
And safer!
# XXX (wdn): I don't think I have a type I can put in here | ||
# without some robot yelling at me | ||
meaningful_history: list[Any] = [] | ||
for h in history: |
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 guaranteed to be ordered?
mh.save() | ||
|
||
except Exception as e: | ||
logging.exception(f"Could not get history for this link: {e}") |
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.
logging.exception(f"Could not get history for this link: {e}") | |
logging.exception(f"Error cleaning history for object: {m}. {e}") |
You know what? I don't think I want to reinvent the wheel here. The process of copying a backup to my laptop and cleaning it there was pretty painful, but there's no way I'd use this in production. |
This management command will erase empty history entries from the mentioned devices. This was created by a bug in the UISP import script, causing it to save unnecessarily, which causes extra writes.