Skip to content

Conversation

sanjaysrikakulam
Copy link
Contributor

In EU @bgruening observed that the brand_by_host gets sorted in the final configuration when written to disk (galaxy.yml file). As a result of this sorting, the usegalaxy.eu: Europe comes before the subdomain virology.usegalaxy.eu: Virology, which leads to the misbranding of the subdomain https://virology.usegalaxy.eu. This subdomain contains the brand Europe instead of Virology.

Example (final config from galaxy.yml):

    brand_by_host:
        africa.usegalaxy.eu: Africa
        annotation.usegalaxy.eu: Genome Annotation
        ...
        ...
        ...
        usegalaxy.eu: Europe
        virology.usegalaxy.eu: Virology

By default, sort_keys is set to true in the to_nice_yaml function in the galaxy.yml.j2 template. Sorting the brand_by_host in Galaxy config affects the branding of the subdomains. Therefore, this PR disables the default sort_keys.

@natefoo, I am unsure whether the default sorting is needed for anything in Galaxy configuration. Please correct me if this is not the case.

By default, sort_keys is set to true in the to_nice_yaml function. Sorting the brand_by_host affects the branding of the subdomains, especially when the root domain is added before the subdomain brand due to the sorting.
@sanjaysrikakulam
Copy link
Contributor Author

ping @natefoo

jdavcs
jdavcs previously approved these changes Sep 23, 2025
Copy link
Member

@jdavcs jdavcs left a comment

Choose a reason for hiding this comment

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

Looks good to me. Sorry for the delayed review!

@jdavcs jdavcs dismissed their stale review September 23, 2025 21:31

On second thought, I have a concernt about this change. I'll post a comment separately.

@jdavcs
Copy link
Member

jdavcs commented Sep 23, 2025

I don't think this is the right solution. This unsets the default sort for all galaxy config properties. We don't view the generated config often, but when we do, it helps when those options are sorted.
This is not a full -1: I'm OK with this if this is the only reasonable fix.
@bgruening is this still an issue? (checking because it's been a year; we should've reviewed this a long time ago - sorry!)

@bgruening
Copy link
Member

@jdavcs its not solved, you see it here: https://virology.usegalaxy.eu/

@natefoo
Copy link
Member

natefoo commented Sep 25, 2025

If disabling sorting does not break idempotency this is probably ok, although I would prefer this would be configurable.

IMO the real solution is for Galaxy to read these config options correctly. It should not be matching usegalaxy.eu for virology.usegalaxy.eu. There is an issue for this at galaxyproject/galaxy#17664.

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

Successfully merging this pull request may close these issues.

4 participants