Improvements for ssh file sources by bernt-matthias · Pull Request #21646 · galaxyproject/galaxy

bernt-matthias · 2026-01-22T14:54:49Z

Requires #22336

disable keepalive messages (why should we keep the connection alive)
- while testing on my instance I had sometimes keepalive messages paramiko.transport DEBUG 2026-01-21 17:29:52,800 [pN:main.1,p:3872036,tN:Thread-8] Sending global request "keepalive@lag.net" every 10s for 2h. I could not reliably reproduce it, but I guess we do not need to keep connections alive, or?
make user mandatory: the default is the current user (which should be the Galaxy user)
allow to use them in templates
switch to fsspec
tests
add possibility for multiline fields in template UI (for input of private keys)

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.

jmchilton · 2026-01-27T18:57:27Z

Why are you making the user mandatory here? You mention the default in your PR description but you don't let it be set to None?

xref https://github.com/althonos/fs.sshfs/blob/master/fs/sshfs/sshfs.py#L36

lib/galaxy/files/templates/examples/ssh.yml

bernt-matthias · 2026-01-30T21:14:23Z

Can't get it working yet. When creating a file source from the template, the I get the error that the path is empty. Any idea @davelopez?

lib/galaxy/files/templates/models.py

bernt-matthias · 2026-02-08T18:03:46Z

Why are you making the user mandatory here? You mention the default in your PR description but you don't let it be set to None?

xref https://github.com/althonos/fs.sshfs/blob/master/fs/sshfs/sshfs.py#L36

In the Galaxy context I found it not user friendly (and a bit "dangerous") to use the default implemented in sshfs: which is the account of the user running the Galaxy webserver. I can't see a scenario where this is useful for a Galaxy user - so I thought it's better to require users to set a username explicitly.

Edit: Guess my formulation was misleading: With "Galaxy user" I did not mean the user, but the user running the Galaxy server.

davelopez · 2026-02-12T10:49:19Z

Now that #21704 is merged into dev, I suggest rebasing this and trying it out.
Remember that now we need to explicitly annotate optional fields with optional: true in the template.

It will also be cool if we could migrate this file source to fsspec as Nicola pointed out here #21823 (comment). But that can be a follow-up PR.

Let me know if I can help with something.

bernt-matthias · 2026-02-13T11:48:44Z

Now that #21704 is merged into dev, I suggest rebasing this and trying it out. Remember that now we need to explicitly annotate optional fields with optional: true in the template.

Will do

It will also be cool if we could migrate this file source to fsspec as Nicola pointed out here #21823 (comment). But that can be a follow-up PR.

I'm certainly interested. I planned to learn more about file sources and their implementation in Galaxy (I have 1-2 that I would be interested to add). Are there any docs that could help me with the migration? Otherwise, maybe we can setup a meeting and you explain me a bit what is needed for the migration.

davelopez · 2026-02-13T12:21:53Z

Are there any docs that could help me with the migration? Otherwise, maybe we can setup a meeting and you explain me a bit what is needed for the migration.

Sure! Let me know, and we can have a meeting to go through the examples or some of the details.

In general, the best documentation right now is the file sources already migrated to fsspec and the base class. In most cases, the migration or implementation is just a matter of defining well-typed configuration models and template configuration models and then doing the wiring with the base class, possibly overriding some of the base methods and functions as needed.

These are the relevant files:

Base Implementation

FsspecFilesSource

Concrete Implementations

bernt-matthias · 2026-02-24T13:04:46Z

Added the fsspec implementation of the ssh filesource from here: https://github.com/bernt-matthias/galaxy/tree/sshfs-fsspec

I wont have time to work on tests ... If anyone wants to jump in? The unit tests worked locally for me (just need to fill the info in the yml file)

davelopez · 2026-03-02T17:24:25Z

I made some extra improvements, and Claude convinced me to write an in-memory ephemeral SFTP server to test instead of relying on local testing or Docker.
If that is too much, we can just drop the tests entirely since most remote file sources are not very test-friendly anyway.

lldelisle · 2026-03-06T11:32:34Z

I look forward to have this implemented, this looks very interesting.

davelopez · 2026-03-20T10:46:23Z

This one should be ready to review. I can't approve since I contributed substantial changes 😅

bernt-matthias · 2026-03-20T10:49:44Z

Thanks for taking over @davelopez

lib/galaxy/files/sources/ssh.py

Moves `paramiko` imports and server implementation classes directly into the `sftp_server` pytest fixture. This change ensures that the `paramiko` library is only loaded when the fixture is actively used, making the test suite more modular and less prone to import errors if `paramiko` is not installed. Adds a skip mechanism to the fixture, allowing tests that depend on the SFTP server to be automatically skipped if `paramiko` is unavailable, improving compatibility with diverse testing environments.

Prevents an `AttributeError` when `paramiko` attempts public-key authentication with a raw private key string instead of a `PKey` object. The code now explicitly sets the `pkey` parameter to `None` if the configured value is `None` or the string "None". The example SSH file source template no longer includes the `pkey` option, preventing users from attempting to provide raw key content incorrectly.

Relocates the SSH file source tests from the unit test suite to a new Selenium integration test. This change extracts the in-process SFTP server setup into a re-usable `SFTPServerMixin`. The new integration test then uses this mixin to verify the SSH file source functionality through the Galaxy UI, ensuring a more comprehensive end-to-end test.

Validates the proper functionality of subdirectory-scoped SSH file sources.

Since paramiko is always co-installed with fsspec[sftp]

davelopez · 2026-04-05T12:46:29Z

I've rebased again to resolve the conflict. In my local tests with an open-SSH Docker instance, it seems to work :)

mvdbeek · 2026-04-07T13:44:20Z

test/unit/files/conftest.py

+
+
+@pytest.fixture()
+def sftp_server() -> Generator[SftpServerInfo, None, None]:


Is this fixture used anywhere ? Previous attempt ?

You're right, it was a leftover. I removed it 👍

github-actions bot added this to the 26.1 milestone Jan 22, 2026

davelopez reviewed Jan 28, 2026

View reviewed changes

lib/galaxy/files/templates/examples/ssh.yml Outdated Show resolved Hide resolved

davelopez reviewed Jan 28, 2026

View reviewed changes

lib/galaxy/files/templates/examples/ssh.yml Outdated Show resolved Hide resolved

davelopez reviewed Feb 2, 2026

View reviewed changes

lib/galaxy/files/templates/models.py Show resolved Hide resolved

bernt-matthias force-pushed the ssh-fs-improvements branch from 83b9b3d to 2ccc586 Compare February 24, 2026 13:02

bernt-matthias mentioned this pull request Feb 24, 2026

Migrate ssh file source plugin to fsspec #21867

Open

bernt-matthias force-pushed the ssh-fs-improvements branch from 9878e98 to 7eb6a42 Compare February 26, 2026 08:56

davelopez marked this pull request as draft March 2, 2026 16:01

davelopez marked this pull request as ready for review March 2, 2026 17:15

github-actions bot added the area/testing label Mar 2, 2026

davelopez force-pushed the ssh-fs-improvements branch from 3088ce2 to 33b4dda Compare March 2, 2026 17:20

davelopez marked this pull request as draft March 3, 2026 12:20

davelopez force-pushed the ssh-fs-improvements branch from 88b1d21 to 419aa7f Compare March 3, 2026 14:41

davelopez marked this pull request as ready for review March 3, 2026 17:35

bernt-matthias commented Mar 20, 2026

View reviewed changes

lib/galaxy/files/sources/ssh.py Outdated Show resolved Hide resolved

bernt-matthias mentioned this pull request Mar 21, 2026

[26.0] File source templates can not be removed #22208

Closed

bernt-matthias marked this pull request as draft March 21, 2026 19:31

bernt-matthias force-pushed the ssh-fs-improvements branch from 36a82f8 to bba0173 Compare March 23, 2026 10:47

davelopez force-pushed the ssh-fs-improvements branch from 9209758 to 92fcea1 Compare March 27, 2026 17:54

bernt-matthias and others added 26 commits April 5, 2026 14:02

use fsspec for ssh file source

f47ad5c

add rudimentary tests

d0a1660

fix linter problems

421b7ac

add ssh to template

a7089c9

more py linter

39e498c

fix schema

877adc1

black

801a120

Makes SSH user field optional in configuration models

92ac8b9

Updates SSH example template to reflect optional parameters

c473b7e

Improves SSH filesystem argument passing and path formatting

52491f5

Adds SSH file source type description to client UI

93781f0

Removes unsupported configuration from SSH file source

cdf6809

Adds ephemeral SFTP server fixture for SSH tests

9d92a79

Refactors SSH file source tests with SFTP fixture

3ad1e9c

Fix linting

de62a7a

Remove leftover import

5c4674c

A new selenium integration test

41a0467

Validates the proper functionality of subdirectory-scoped SSH file sources.

Make SFTP import conditional

5a9d1b4

implement private key handling

d7e4418

Add multiline secret support to config templates

fe997dc

Refactor SSH private key handling for multiline secrets

93a187a

Raise error on invalid SSH private key during auth

29d3664

Defers paramiko key imports

9054fbe

Since paramiko is always co-installed with fsspec[sftp]

davelopez force-pushed the ssh-fs-improvements branch from 8ea1e53 to 9054fbe Compare April 5, 2026 12:05

mvdbeek reviewed Apr 7, 2026

View reviewed changes

Remove unused fixture

89999eb



		@pytest.fixture()
		def sftp_server() -> Generator[SftpServerInfo, None, None]:

Conversation

bernt-matthias commented Jan 22, 2026 • edited by davelopez Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to test the changes?

License

Uh oh!

jmchilton commented Jan 27, 2026

Uh oh!

Uh oh!

Uh oh!

bernt-matthias commented Jan 30, 2026

Uh oh!

Uh oh!

bernt-matthias commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davelopez commented Feb 12, 2026

Uh oh!

bernt-matthias commented Feb 13, 2026

Uh oh!

davelopez commented Feb 13, 2026

Base Implementation

Concrete Implementations

Uh oh!

bernt-matthias commented Feb 24, 2026

Uh oh!

davelopez commented Mar 2, 2026

Uh oh!

lldelisle commented Mar 6, 2026

Uh oh!

davelopez commented Mar 20, 2026

Uh oh!

bernt-matthias commented Mar 20, 2026

Uh oh!

Uh oh!

davelopez commented Apr 5, 2026

Uh oh!

mvdbeek Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

davelopez Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bernt-matthias commented Jan 22, 2026 •

edited by davelopez

Loading

bernt-matthias commented Feb 8, 2026 •

edited

Loading