Skip to content
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

Drop database views (alt. implementation) #15888

Closed
wants to merge 5 commits into from

Conversation

jdavcs
Copy link
Member

@jdavcs jdavcs commented Mar 31, 2023

Alternative implementation for #15876

This is a straightforward refactoring of HistoryDatasetCollectionJobStateSummary's implementation with minimal change:

  • The view is replaced with a subquery (ref: example of such mapping in SA's docs)
  • The structure of the original view's query is left unchanged, but the query itself is implemented in SQLAlchemy's SQL Expression Language.
  • The model.views directory is dropped.

NOTE: The part of the query that enumerates the job states (aggregate sum columns) is now generated dynamically based on members of model.Job.states. This was done as a safeguard against missing a state (assertion added) or listing a non-existent state (e.g. the original view's query had a bug: the state deleted_new has been replaced with deleting, which may have resulted in incorrect counts.)

(Rationale for dropping views: see #15876)

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 jdavcs added kind/refactoring cleanup or refactoring of existing code, no functional changes area/database Galaxy's database or data access layer labels Mar 31, 2023
@jdavcs jdavcs added this to the 23.1 milestone Mar 31, 2023
@jdavcs jdavcs mentioned this pull request Mar 31, 2023
4 tasks
@jdavcs
Copy link
Member Author

jdavcs commented Mar 31, 2023

API test failures are relevant.

@jdavcs
Copy link
Member Author

jdavcs commented Apr 10, 2023

Closing in favor of #15876

@jdavcs jdavcs closed this Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/database Galaxy's database or data access layer kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant