Replace --disable-clean with --enable-clean. #2305
Conversation
| @@ -389,7 +389,7 @@ def get_args(): | |||
| help=('Get all details about gyms (causes an ' + | |||
| 'additional API hit for every gym).'), | |||
| action='store_true', default=False) | |||
| parser.add_argument('--disable-clean', help='Disable clean db loop.', | |||
| parser.add_argument('-DC', '--enable-clean', help='Enable DB cleaner.', | |||
There was a problem hiding this comment.
-DC for --enable-clean seems confusing (is it meant to be short for "DB Cleaner"?)
There was a problem hiding this comment.
+1 and caps are also weird
There was a problem hiding this comment.
@michikrug @friscoMad Correct, -D is used for the database (although currently sqlite). -DC is short for the DB's cleaner. Caps aren't weird, they're common for differences in behavior.
If they're confusing you because you're reading the short and long form together, consider the contexts of their use cases: you're setting cli args, you think of -D for the database and also remember you want to enable the cleaner, so -DC will be easy to remember. For more detailed configuration in the config file, you have the clean long form --enable-clean.
We currently only have long forms for database info (e.g. --db-host) which could get a short form by extending the -D argument: -DH for host (and we have -H for web host), -DP for port, and so on.
For the future, this is what we're going for. It avoids absurd and arbitrary short forms - pxo, ldur, wwhtf, ...
There was a problem hiding this comment.
I don't see a problem. -DatabaseCleaner -> -DC. Especially if we're considering extending the database args like that for -DH, -DP, etc. Prefixing arguments generally makes sense and should allow people to make educated guesses. (Oh, I need to change something in the database... -D... and I need to change the port... P... -DP...)
There was a problem hiding this comment.
I found it confusing as we already have -dc and because the previous behaviour was --disable-clean so -DC also seems to be a shorthand of it, and I do not see a problem to use D as a prefix for all database options or P for proxies but in that case -Dec and --db-enable-clean seems a better fit for me or if we prefrer a shorter version -Dc and --db-clean
Description
Reverse the behavior for the config item of the background database cleaner.
Motivation and Context
The DB cleaner should only be enabled on one instance, which fits better with
-DC, --enable-clean. Too many people skipped--disable-clean, enabling the DB cleaner on all their instances and inevitably running into MySQL deadlocks.Types of changes
Checklist: