-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Implement instance URLs in Galaxy markdown. #16675
Conversation
5ecf2f5
to
8ff3702
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instance_organization_link()
- We already had configuration values calledga4gh_service_organization_name
andga4gh_service_organization_url
that we needed to expose for GA4GH APIs. All the APIs use similar service endpoint descriptions. I added new configuration values that are more general calledorganization_url
andorganization_name
for building links that describe the organization hosting Galaxy. The GA4GH values can still be set to more specific things (i.e. if you wanted to define "Evil Corp" as the organization and "Evil Corp - GA4GH Department" as the service provider) but setting the organization-level values make the most sense 99% of the time I think.
Can we add the ga4gh_service_organization_*
options to GalaxyAppConfiguration.renamed_options
instead of keep supporting them? The GA4GH Department use case seems very theoretical to me.
I hate that. Really feels like I'm being punished for not putting more time into the specifics examples of the documentation. I will make the change though. |
The goal of my comments is to keep the number of new configuration options as low as possible, to reduce the cognitive load of admins and keep things more maintainable for developers, not to punish you! |
I would rather hide the options in the documentation if that is your concern than eliminate the knobs. When I do reviews, I generally approve things I disagree with and make it clear I'm okay with changes I dislike unless it is something serious. Even then, I try to make it clear if it is a -0 or a -.9 or a -1. To your point though - my goal is in fact to discourage the request for changes to PRs if I don't agree with them - I have an agenda for the project I am trying to advance. I don't feel like I have a clear outlet for dissent on a review like this other than to express my disappointment. You're wielding power you've earned and I'm going to submit to it. I try to keep it professional and not personal or directed at you - if I were to make personal directed comments about you - I would say Nicola is brilliant and dedicated and meticulous and an irreplaceable and invaluable asset to the project. |
a9261a3
to
24facf5
Compare
5f4334f
to
2b219fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
2b219fe
to
c1d3eb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks, @jmchilton!
Working my way through some of the ideas on #16556.
The request was for "current citation for the Galaxy instance it ran on". I think there is a bunch of different things along these lines that would all be useful depending on the application. See the comments on the above issue for more discussion. This adds PR adds 7 new Galaxy markdown link directives - these are for the Galaxy URL, the organization URL, the citation URL, the support URL, the help URL, the terms URL, and the "resources URL" (what we call galaxyproject.org for main).
Most of these are already defined in Galaxy and we just reuse what is there - I did create one new config option and sort of reworked, generalized two existing ones to support this. This is as discussed below:
instance_resources_link()
- We define the config value in config_schema.yml calledinstance_resource_url
and we use it for linking to galaxyproject.org in activation e-mails. This seems like a reasonable configuration value to re-use for reports/pages and expose for general statements about learning more about Galaxy and such.instance_access_link()
- I addedinstance_access_url
to mirrorinstance_resource_url
and serve as a way for admins to configure the endpoint URL they want to use for accessing the Galaxy instance. For main, I assume we'd set this tohttps://usegalaxy.org
.instance_organization_link()
- We already had configuration values calledga4gh_service_organization_name
andga4gh_service_organization_url
that we needed to expose for GA4GH APIs. All the APIs use similar service endpoint descriptions. I added new configuration values that are more general calledorganization_url
andorganization_name
for building links that describe the organization hosting Galaxy. The GA4GH values can still be set to more specific things (i.e. if you wanted to define "Evil Corp" as the organization and "Evil Corp - GA4GH Department" as the service provider) but setting the organization-level values make the most sense 99% of the time I think.instance_citation_link()
- Already used in the UI, set viacitation_url
in Galaxy's config, and defaults to .https://galaxyproject.org/citing-galaxy
instance_support_link()
- Already used in the UI, set viasupport_url
in Galaxy's config, and defaults to https://galaxyproject.org/support/ .instance_help_link()
- Already used in the UI, set viahelpsite_url
in Galaxy's config, and defaults to https://help.galaxyproject.org/.instance_terms_link()
- Already used in the UI, set viaterms_url
in Galaxy's config, and has no default.The inputs:
The result:
The PDF:
These are sort of niche - so I don't expand the toolbox section with these link directives by default - but they are available if you expand that section:
The config.yaml entries I added for this:
How to test the changes?
(Select all options that apply)
License