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

Bring your own file sources: Add the WebDAV template and configuration #18598

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

sanjaysrikakulam
Copy link
Contributor

This PR adds the

  1. WebDAV example template and configuration
  2. Updates the docs, the file source model, and the client API schema.

ToDo:

  1. Generate diagrams and images for the docs

@bgruening
Copy link
Member

@sanjaysrikakulam there is a make target that helps with the lining

@jmchilton can this go to 24.1?

@bgruening bgruening requested a review from jmchilton July 25, 2024 10:06
@sanjaysrikakulam
Copy link
Contributor Author

@sanjaysrikakulam there is a make target that helps with the lining

Cool, thanks! Unfortunately, I don't have the Galaxy environment. It's taking forever to create it due to some network error,

Example:

WARNING: Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.HTTPSConnection object at 0x7ff3b44af700>: Failed to establish a new connection: [Errno -2] Name or service not known')': /simple/cwl-upgrader/

Hence, the reason for the draft. I am unable to generate the diagrams and images at the moment.

@davelopez davelopez requested a review from a team September 19, 2024 13:19
@volodymyrss
Copy link
Contributor

With prior advice of @bgruening , I just want to stress that this feature is very important for us in astro(particle)physics!

lib/galaxy/config/__init__.py Show resolved Hide resolved
lib/galaxy/config/sample/galaxy.yml.sample Outdated Show resolved Hide resolved
lib/galaxy/files/sources/webdav.py Outdated Show resolved Hide resolved
from typing import (
Optional,
Union,
)

from galaxy import config
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this will work 🤔 this is importing the config module, but I guess you need an instance of the app config with all the processed values available in the _open_fs method.

Maybe @mvdbeek or @jmchilton know how to reference the configuration values here, I don't know a good way of doing that but I might be missing something 😞

Copy link
Member

Choose a reason for hiding this comment

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

Just don't do it. Parse the config values in the config, this is too late. props as serialized should contain all values ready to go.

Copy link
Member

Choose a reason for hiding this comment

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

I'll have a look at this.

Copy link
Member

Choose a reason for hiding this comment

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

I think #18909 is a version of what I was hoping for. Can we rebase this PR after that one is merged? Might be worth re-testing it on your end after that as well. Thanks for you patience - sorry to be a menace about this - I'm just very anxious about what we put in the database.

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries John, we just want to do it as best as we can :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @jmchilton! Yes, I can rebase and fix this PR accordingly and would be happy to test it.

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

Successfully merging this pull request may close these issues.

6 participants