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

Job execution notification emails do not generate accurate URLs for jobs. #18747

Closed
lparsons opened this issue Aug 28, 2024 · 2 comments
Closed

Comments

@lparsons
Copy link
Contributor

lparsons commented Aug 28, 2024

Describe the bug
When an email is sent to notify the user that a job has completed, the link to the job uses the galaxy_infrastructure_url, which is not always appropriate. Instead, it should use url_for.

Galaxy Version and/or server at which you observed the bug
Galaxy Version: 21.05 (expect it to exist on 24.1 and up as well)

Browser and Operating System
Operating System: Windows, Linux, macOS (all)
Browser: Firefox, Chrome, Chrome-based, Safari (all)

To Reproduce
Steps to reproduce the behavior:

  1. Run a job and select email notification when complete
  2. The URLs in the email will be to localhost or whatever galaxy_infrastructure_url is set to.

Expected behavior
The URL should use the appropriate server address. The galaxy_infrastructure_url is only intended to be used by internal services.

Additional context
The post execution job action uses galaxy_infrastructure_url.

link_invocation = (
f"{app.config.galaxy_infrastructure_url}/workflows/invocations/report?id={invocation_id_encoded}"
)
link = f"{app.config.galaxy_infrastructure_url}/histories/view?id={history_id_encoded}"

Instead, it should use url_for which is done in the error reporting emails.

history_view_link = self.app.url_for("/histories/view", id=history_id_encoded, qualified=True)
hda_id_encoded = self.app.security.encode_id(hda.id)
hda_show_params_link = self.app.url_for(

@mvdbeek
Copy link
Member

mvdbeek commented Aug 28, 2024

Hey @lparsons, that is not possible unfortunately, url_for only works in a web context that is not available in the job and workflow handlers where these are used. The error report emails are handled within the web context, that's why they can use url_for. This is the relevant work to add another variable for this purpose #18576.

Realistically there's only a problem with galaxy_infrastructure_url if it is set to a truly internal URL for ITs or pulsar that is distinct from the public fqdn.

@lparsons
Copy link
Contributor Author

Thanks @mvdbeek. I didn't realize the issue with using url_for, that's too bad. We never had galaxy_infrastrucutre_url set since it didn't seem necessary, and the default is technically correct (localhost), however, it caused the links to be broken. I guess going forward we'll just make sure to set it to the public URL instead.

@mvdbeek mvdbeek closed this as completed Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants