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.

Sign up for GitHub

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

Jump to bottom

Migrate Library Contents API to FastAPI #18838

Merged
merged 77 commits into from
Oct 7, 2024
Merged

Migrate Library Contents API to FastAPI #18838

merged 77 commits into from
Oct 7, 2024

Conversation

arash77
Copy link
Collaborator

@arash77 arash77 commented Sep 17, 2024

What did you do?

  • Include an API test under lib/galaxy_test/api/
  • Refactor the legacy library_contents API logic so all the logic is contained in the LibraryContentsService class
  • Create pydantic models for the responses and payloads
  • Create the FastAPI routes that will replace the old ones

related to #10889

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.

@arash77 arash77 marked this pull request as ready for review September 26, 2024 12:46
@github-actions github-actions bot added this to the 24.2 milestone Sep 26, 2024
@arash77
add LibraryFolderDatabaseIdField Model for library_id
07db06a
@arash77
Fix schema for uploaded_by being None in show library contents endpoi…
c0e589b 
…nt caused in test_materialize_library_dataset
@arash77
Add function for supporting files in library_contents
9e47626
@arash77
remove unused enumerate and files field into LibraryContentsFileCreat…
3e22733 
…ePayload
@arash77
Refactor for better readability
6d59a46
@arash77
send data as json when there is no files on creating new_library_data…
d43e6b8 
…set for tests
@arash77
update files to upload_files in create payload
4d30692
@arash77
remove unused import
9d0bd72
@arash77
send none files library_contents create in population with json
f4cffb3
@arash77
Apply suggestions: s/title/description in schema, don't do try/except…
0a86966 
… in delete logic, rename type aliases for consistency
@arash77
remove unused import
d7c3b81
@arash77
update schema
89a056e
@arash77
Merge branch 'LC_test' into FastAPI_library_contents
8d417fb
@arash77
schema linting
b03ce07
@arash77
Using DecodedDatabaseIdField instead of `Union[DecodedDatabaseIdFie…
110e467 
…ld, LibraryFolderDatabaseIdField]` for library_id and fix `_create_dataset_in_folder_in_library` function that is leading to failed tests
arash77 and others added 2 commits October 7, 2024 12:23
@arash77 @mvdbeek
Remove log from library_contents
5cac3e9 
Co-authored-by: Marius van den Beek <m.vandenbeek@gmail.com>
@arash77
move library_contents async part into a dependency
73ff5c6
@mvdbeek mvdbeek merged commit 4975836 into galaxyproject:dev Oct 7, 2024
55 of 56 checks passed
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 7, 2024

This PR was merged without a "kind/" label, please correct.

@mvdbeek mvdbeek added the kind/refactoring cleanup or refactoring of existing code, no functional changes label Oct 7, 2024
@arash77 arash77 deleted the FastAPI_library_contents branch October 7, 2024 14:37
@arash77
Copy link
Collaborator Author

arash77 commented Oct 7, 2024

Thank you for the reviews. @davelopez @mvdbeek

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvdbeek mvdbeek mvdbeek left review comments

@davelopez davelopez davelopez left review comments

Assignees
No one assigned
Labels
area/API area/testing/api area/testing kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Milestone
24.2
Development

Successfully merging this pull request may close these issues.
3 participants
@arash77 @davelopez @mvdbeek