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

Allow path-based interactive tools using nginx proxy #14694

Merged
merged 4 commits into from
Oct 17, 2022

Conversation

sveinugu
Copy link
Contributor

Fixed issue causing the path of the path-based interactive tool entry point IRI to begin with two slashes (when interactivetools_base_path has the default value /). Additionally added documentation for how to set up path-based interactive tools using a nginx proxy (while we are waiting for a fix for stand-alone galaxy deployments, see: #14690)

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.
  • [ x ] Instructions for manual testing are as follows:
    1. Set up a plain, stand-alone Galaxy with the configuration described in galaxy.yml.interactivetools and job_conf.xml.interactivetools.
    2. Run an interactive tool with requires_domain=False (which is a bit cumbersome, as I don't think any of the bundled ones support that).

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@sveinugu
Copy link
Contributor Author

Did not find existing tests of InteractiveToolManager. I don't have time to write one.

@sveinugu
Copy link
Contributor Author

Also thx to @morj-uio for making this work

Copy link
Member

@mvdbeek mvdbeek left a comment

Choose a reason for hiding this comment

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

Thanks for working that out!

@mvdbeek
Copy link
Member

mvdbeek commented Sep 27, 2022

(while we are waiting for a fix for stand-alone galaxy deployments, see: #14690)

Do you think that'll be necessary ? domain-based ITs are working with a local instance, and I don't think Galaxy should enter the field of being a proxy.

@sveinugu
Copy link
Contributor Author

sveinugu commented Sep 27, 2022

(while we are waiting for a fix for stand-alone galaxy deployments, see: #14690)

Do you think that'll be necessary ? domain-based ITs are working with a local instance, and I don't think Galaxy should enter the field of being a proxy.

When you say that domain-based ITs are working with a local instance, are you referring to the localhost.blankenberglab.org solution described in the docs? IMO, this is more of a hack than a solution. I have not actually tried this, as I am mainly developing towards a deployed Galaxy instance, so it might be working just fine. However, being dependent on that DNS entry is at least not a standalone solution.

The main logical problem is this:

  1. Path-based Interactive Tools require changes to the underlying docker image to work. Most IT docker images have not made these (or are at least not tested for path-based use).
  2. For local development, only domain-based ITs work using the localhost.blankenberglab.org hack (at least as far as I am aware).
  3. Domain-based ITs are a major showstopper in deployment, at least for smaller Galaxies and in restricted situations (such as cloud deployments etc). See Path-based interactive tools stopped working in 22.05 #14690 (comment) from @hexylena.
  4. Since 22.05, there is no way to get path-based Interactive Tools to work locally, which makes it very hard to fix 1, and consequently 3.

With gx-it-proxy, Galaxy is already in the proxy business. One possible solution might be to update that proxy to also handle path-based URLs. In addition, if one could configure Galaxy to add a port number to the path-based urls, I think we are in business. If this solution makes better sense than any alternatives, I can add an issue there. I don't know who is primarily maintaining that repo, though, if any.

@mvdbeek
Copy link
Member

mvdbeek commented Sep 27, 2022

When you say that domain-based ITs are working with a local instance, are you referring to the localhost.blankenberglab.org solution described in the docs?

Here's the current docs: https://docs.galaxyproject.org/en/release_22.05/admin/special_topics/interactivetools.html#server-side-configuration-of-galaxy-interactivetools, we simply use localhost.

I don't quite see what's wrong with running nginx with the docs you've outlined here, but sure, if you can figure out a solution with https://github.com/galaxyproject/gx-it-proxy we're happy to merge that.

There's some stalled work on supporting traefik for ITs, but at the time it seemed that you'd still need a proxy upstream, so I'm not sure that's the right direction.

@sveinugu
Copy link
Contributor Author

Right. I missed that. This is clearly an improvement over the previous hack! Not sure exactly how this works, but it will in any case only work for the domain-based URLs for now. So setting up nginx on your laptop to be able to test out path-based ITs is at least not the right tool for the job. Improving gx-it-proxy seems to be the correct option, then, and it seems there isn’t any need to do anything on the Galaxy side.

I’ll see what I can do if no-one else beats me to it, but it will have to wait a bit as we have now gotten our path-based IT up to run again on the deployed server using nginx, which was our immediate concern.

@sveinugu
Copy link
Contributor Author

Right. I missed that. This is clearly an improvement over the previous hack! Not sure exactly how this works, but it will in any case only work for the domain-based URLs for now. So setting up nginx on your laptop to be able to test out path-based ITs is at least not the right tool for the job.

Thinking a bit further, I see now that I have been a bit confused. As I was thinking that one had to use the localhost.blankenberglab.org domain to run ITs on a laptop, one would then not be able to test whether an IT would work on localhost. However, the new config you pointed me towards will use localhost and not a domain. So does that mean that if an IT works on a laptop with Galaxy configured this way, one can expect this IT to also work in production under path-based URLs?

To put it simply: Can one conclude that if one is able to run an IT with requires-domainset to True on a laptop with the configuration you referred to, then one could switch the requires-domain setting for that IT to False?

@mvdbeek mvdbeek merged commit 672ed49 into galaxyproject:dev Oct 17, 2022
@mvdbeek
Copy link
Member

mvdbeek commented Oct 17, 2022

Thanks @sveinugu, hoping you and @hexylena can find a way to better support for path based ITs!

@github-actions
Copy link

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

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.

2 participants