Refactor/cleanup admin_cleanup_datasets.py script

[ccoulombe]

Update the admin_cleanup_datasets.py script to work with SQLAlchemy 2.x. Plus a few little updates.

Changes:

• Nicer time format [b6f5553]
• Update to work with SA 2.x [3904721]
• Refactored and updated the administrative_delete_datasets function to be compatible with SA 2.x, and easier to the eye [860e2cd]
• Refactored and updated the _get_tool_id function to be compatible with SA 2.x and easier to the eye [651426a]
• Add state of deletion to email subject [f0e63ff]
• Add option to not send the email upon deletion [85d8e6a]

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. On the galaxy server, export $GALAXY_CONFIG_FILE to the galaxy config, activate the virtual environment, move to the <root>/server/scripts/cleanup_datasets directory
  2. Create dummy datasets, and create one with a specific tool
  3. Run python admin_cleanup_datasets.py --days 0 --info_only --config $GALAXY_CONFIG_FILE --template admin_cleanup_warning_template.txt.sample
  4. Run python admin_cleanup_datasets.py --days 0 --info_only --config $GALAXY_CONFIG_FILE --template admin_cleanup_warning_template.txt.sample --tool_id <id>
  5. Run python admin_cleanup_datasets.py --days 0 --email_only --config $GALAXY_CONFIG_FILE --template admin_cleanup_warning_template.txt.sample --tool_id <id> and check for the email containing the datasets from the tool that generated them
  6. Run python admin_cleanup_datasets.py --days 0 --config $GALAXY_CONFIG_FILE --template admin_cleanup_warning_template.txt.sample --tool_id <id> and check for deleted datasets
  7. Run both versions of the scripts and compare their output

[jmchilton]

Can you run "make format" - this looks solid to me but the linter is unhappy about code formatting.

[jdavcs]

deleted.is_(False) is identical to deleted == false(). In the rest of the codebase we use the former. We use the is_ construct when comparing to null. Let's keep false() for consistency.

But replacing Foo.__table__.c.bar with Foo.bar is correct.

[jdavcs]

We don't need aliased classes here. If you want to improve the readability of the code, you can import the classes from the model and then use just the class names:

from galaxy.model import HistoryDatasetAssociation
my_statement = select(HistoryDatasetAssociation).where(whatever...)

An extra benefit of this is that reading (and grepping) the code is easier: the string HistoryDatasetAssociation will always represent the same thing across the codebase.

[jdavcs]

Let's not delete this: this is a potentially useful comment which, maybe, hasn't been addressed yet. Yes, it's 12 years old, but it still could be helpful. (jtod and copied_from refer to database tables). Would be OK to delete if this particular item were addressed and deemed no longer relevant.

[jdavcs]

This changes the function's behavior: the previous version would raise an error here, which is correct.
Also, we don't need to check that an hda exists here.

[jdavcs]

We don't need to specify the join criteria here: SQLAlchemy takes care of it for us.

[jdavcs]

We don't need the join criteria.

Very nice refactoring here - thanks!

[jdavcs]

Same as above - this is very nice refactoring, thanks!

[ccoulombe]

@jmchilton Yes, will do.
@jdavcs Thanks for the comments, will tackle them.

... once I get back from vacation in a week!