SQLAlchemy 2.0 upgrades (part 4)

[jdavcs]

SQLAlchemy 2.0 compatibility upgrades. Ref #12541.

This covers the managers/ directory, with just a few cases left out (in base, history_contents and sharable - those involve query-building abstractions which may require more refactoring; I'll have a separate PR for those). All commits are atomic; most are straightforward syntactical edits. If there is a notable refactoring present, I noted that in the commit description.

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. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[jdavcs]

Converting to draft. While working on a related session issue caught a failing API test that only happens locally and is SA-related. I don't understand (yet) why it has always passed remotely, so marking this as draft until l figure this out and can verify it works correctly.

[jdavcs]

Yes, so it happens on sqlite only:
test_workflows.py::TestWorkflowsApi::test_invocation_map_over
sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) 1st ORDER BY term does not match any column in the result set.
I'll fix this one and will rerun the entire suite locally - I wonder if there'll be many more failures that have went unnoticed.

[jdavcs]

I've removed 2 commits which handled the index_query method in the Job manager. I'll submit a separate PR for that because that requires special handling. Here's the gist of it: if workflow_id or invocation_id are supplied, the select statement becomes a union. That leads to the following problem: the combination of UNION and ORDER BY don't behave as expected under SQLite, which does not recognize the term create_time as referencing a column in the result set: it requires a table prefix, so job.create_time would work. The legacy Query object handled this by wrapping the union in an additional SELECT, in which case Job.create_time was automatically prefixed with anon_1 - the table name generated by SQLAlchemy. The Select object does not handle this case implicitly. As a result, (1) we get much cleaner SQL, and (2) an error under SQLite.

The solution is relatively simple, but requires some refactoring that, I feel, belongs in a separate PR (it's a different type of fix-up compared to what's done here (which is to only upgrade to SA2.0).

SQL generated by the Select object:

SELECT job.id, job.create_time, ...
FROM job ...
UNION
    SELECT job.id, job.create_time, ...
    FROM job ...
ORDER BY create_time DESC  -- this breaks

SQL generated by the legacy Query object:

SELECT
	anon_1.job_id AS anon_1_job_id,
	anon_1.job_create_time AS anon_1_job_create_time, ...
FROM
	(
		SELECT
			job.id AS job_id,
			job.create_time AS job_create_time, ...
		FROM job ...
		UNION
			SELECT
				job.id AS job_id,
				job.create_time AS job_create_time, ...
			FROM job ...
ORDER BY
	anon_1.job_create_time DESC  -- this works fine

[mvdbeek]

Seems like an exists would be marginally better ?

[jdavcs]

Yes, and easier to read too! I'll add this to the next batch (#16932).
Thanks for reviewing!

[jdavcs]

Addressed in 7769ddc.

[mvdbeek]

Looks great, thank you!