Skip to content

Add fsspec base implementation for File Source plugins #20698

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
Open
wants to merge 9 commits into
base: dev
Choose a base branch
from
Open

Add fsspec base implementation for File Source plugins #20698

wants to merge 9 commits into from

Conversation

davelopez
Copy link
Contributor

@davelopez davelopez commented Jul 25, 2025 •
edited
Loading

This PR adds a base FsspecFilesSource class as an adapter to integrate any fsspec-compatible backend to the Galaxy FilesSources framework.

  • Includes the migration of the TempFilesSource to FsspecFilesSource for testing purposes.
  • Adds a non-production MemoryFilesSource to test the base integration without a real backend.

TODO

  • More testing

xref #20415

How to test the changes?

  • 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.

@davelopez davelopez added kind/enhancement kind/refactoring cleanup or refactoring of existing code, no functional changes area/backend labels Jul 25, 2025
@github-actions github-actions bot added this to the 25.1 milestone Jul 25, 2025
@davelopez davelopez marked this pull request as draft July 25, 2025 14:59
@davelopez davelopez force-pushed the add_fsspec_support branch from 5333af2 to ae3d450 Compare July 28, 2025 14:01
@davelopez davelopez marked this pull request as ready for review July 28, 2025 14:35
@davelopez
Copy link
Contributor Author

I think the basic implementation is ready for review.
I can follow up on improving the testing around file sources and start migrating some of the other plugins, like the s3fs one.

jmchilton
jmchilton approved these changes Jul 28, 2025
View reviewed changes
Copy link
Member

@jmchilton jmchilton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation seems good to me. Since you're working on this component - I wanted to make the case that documentation would be really nice as you're implementing new components. In particular if you could create Pydantic models for the inputs supported by our framework - instead of passing kwds around so much. We could generate documentation for these plugins like we have for the user-defined variants (https://docs.galaxyproject.org/en/master/admin/data.html#file-source-types) and we could be more sure of what needs to be ported when we do have to migrate implementations.

@davelopez
Copy link
Contributor Author

I will happily work on those Pydantic models!
Do you think it would be better to make these changes along with the PR, in a follow-up PR, or even in a PR before implementing these changes?

davelopez added 9 commits July 29, 2025 22:22
@davelopez
Add fsspec to dependencies
c83039e
@davelopez
Implement basic FsspecFilesSource class
3e59fb5 
For file source system integration with fsspec
@davelopez
Enhance FsspecFilesSource
284d8fc 
- Introduced _is_templated method to check if property values are templated.
- Updated get_prop method to evaluate templated properties using user context.
- Enhanced ensure_required_dependency method to print dependency checks.
- Added _initialize_listings_expiry method to manage listings expiry time.
- Improved listing methods with pagination and glob pattern support.
@davelopez
Refactor FsspecFilesSource for improved listing
eff0592
@davelopez
Refactor TempFilesSource to use FsspecFilesSource
3d21ba0 
Since there is not TempFileSystem in fsspec we need to manually box and unbox the paths to a temp folder `root_path` using a regular LocalFileSystem
@davelopez
Add type assertions for TempFilesSource in test fixtures
7b81e49
@davelopez
Implement MemoryFilesSource for testing
260a446 
This mostly to test the the base class without requiring configuration for any real fsspec compatible backend
@davelopez
Fix remote entry serialization in fsspec
8555199
@davelopez
Enhance docs for TempFilesSource
6355ff4
@davelopez davelopez force-pushed the add_fsspec_support branch from ae3d450 to 6355ff4 Compare July 29, 2025 20:22
@davelopez davelopez mentioned this pull request Aug 1, 2025
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmchilton jmchilton jmchilton approved these changes

Assignees
No one assigned
Labels
area/backend kind/enhancement kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Milestone
25.1
Development

Successfully merging this pull request may close these issues.
2 participants
@davelopez @jmchilton