Add script to delete stale records from the DB #7
Conversation
Sae126V
force-pushed
the
stale-summary-clean-up-script
branch
4 times, most recently
from
af5a3fa to
bc8f49c
Compare
Sae126V
force-pushed
the
stale-summary-clean-up-script
branch
from
bc8f49c to
f2e0a02
Compare
|
This PR is ready for the review. If the script requires passing multiple table names at once to delete the data [With the same threshold and timeframe]. I have the solution ready in my local. Please let me know if we want that feature. The feature would be like , separated values. table_name = VNormalisedSummaries, someOtherTableNam |
No, I can't see a use-case for that at the moment. We're only interested it getting one table cleaned up. |
| # type of database | ||
| backend = mysql |
There was a problem hiding this comment.
This isn't actually used, and we know the type of database we're targetting. Remove.
portal-tools/delete_stale_records.py
Outdated
| result = cursor.fetchone() | ||
|
|
||
| if not result or not result[0]: | ||
| no_records_found_error = "No UpdateTime values found. Nothing to purge." |
There was a problem hiding this comment.
Constants should be ALL_CAPS.
There was a problem hiding this comment.
I have removed this as we are no longer using. But, changed the code in other places for ALL_CAPS
portal-tools/delete_stale_records.py
Outdated
| ) | ||
|
|
||
| if args.dry_run: | ||
| print(column_not_found_error) |
There was a problem hiding this comment.
Rather than doing all these ifs for print or log, just configure the logging at the start to output to stdout if it's a dry-run.
There was a problem hiding this comment.
Ahh!. I didn't thought of it. Make sense to me.
portal-tools/delete_stale_records.py
Outdated
| default_config_path = '/etc/apel/delete_stale_records.cfg' | ||
| default_logfile_path = '/var/log/apel/delete_stale_records.log' |
| from datetime import timedelta | ||
| from argparse import ArgumentParser | ||
| from configparser import ConfigParser, NoSectionError, NoOptionError | ||
| import MySQLdb |
There was a problem hiding this comment.
Add a space between built in and 3rd-party imports.
| hostname = | ||
| # port to connect to | ||
| port = 3306 | ||
| # database name |
There was a problem hiding this comment.
I know the DB config for APEL is quite verbose in its comments, but we don't need to repeat that. name should be schema_name to make it more descriptive, and you could keep the comment for table_name but otherwise, the other config options are quite straightforward.
There was a problem hiding this comment.
Sure. name is the one we are using in other places as well which is why I thought to keep it consistent. As this is standalone file it make sense to be more descriptive.
Sae126V
force-pushed
the
stale-summary-clean-up-script
branch
from
ca2ea6e to
3da7c6a
Compare
Resolves GT-1092, GT-1057