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

Factor out DatastoreResource class #4372

Draft
wants to merge 3 commits into
base: 2.x
Choose a base branch
from
Draft

Factor out DatastoreResource class #4372

wants to merge 3 commits into from

Conversation

dafeder
Copy link
Member

@dafeder dafeder commented Dec 27, 2024

Fixes #4359

Describe your changes

Remove DatastoreResource class, use DataResource for everything

QA Steps

  • Add manual QA steps in checklist format for a reviewer to perform. Be as specific as possible, provide examples if appropriate.

Checklist before requesting review

If any of these are left unchecked, please provide an explanation

  • I have updated or added tests to cover my code
  • I have updated or added documentation

@dafeder
Copy link
Member Author

dafeder commented Dec 27, 2024

Currently debugging failing MySqlQueryDownloadControllerTest, which I think just happens to be the first test to run that tries to run a query. It fails at AbstractDatabaseTable::setTable(), I think something is making the perspective be local_file when it should be something else? It can't get a schema anyway.

@dafeder dafeder changed the title Factor our DatastoreResource class Factor out DatastoreResource class Dec 27, 2024
@dafeder
Copy link
Member Author

dafeder commented Dec 30, 2024

Some progress. Need to redo QueryDownloadControllerTest somehow... not so easy to use fake resource names because table name is no longer based on an ID in the DatastoreResource class that we can simply control via the constructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consolidate DataResource and DatastoreResource
1 participant