-
Notifications
You must be signed in to change notification settings - Fork 25
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
Make sure tpv shared db is loaded first #389
Conversation
@@ -65,10 +65,10 @@ execution: | |||
function: map_tool_to_destination | |||
rules_module: tpv.rules | |||
tpv_config_files: | |||
- "https://gxy.io/tpv/db.yml" |
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.
Can we version this as anything else ? Otherwise I'd prefer to just use a git commit. We don't want to update these as we update Galaxy, which has the potential to go wrong. I'm a big fan of one change at a time.
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.
Let's pin it to a commit for now. That way, you can control updates at your pace. In the meantime, we can see what kind of versioning scheme would make sense in the TPV shared db repo.
My own view is that a constant url that people can refer to and always get the latest version has a lot of value. There's just less maintenance that way. TPV relies on testing to ensure that behaviour is consistent. The original idea was that if we introduce any breaking change, the tests will fail, and that's the point at which a new version should be minted. I think what you're looking for is an even greater level of preciseness and control. For now, that can be achieved through commit pinning. In future, we can figure out a good automated release process.
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.
My own view is that a constant url that people can refer to and always get the latest version has a lot of value.
Not saying it's not, but there's a reason one typically doesn't deploy an application from dev. This is fine for dev instances, but not for production.
The original idea was that if we introduce any breaking change, the tests will fail
I like that as an aspirational goal, but tpv-lint doesn't cover enough ground to be confident at this point.
- "{{ tpv_config_dir }}/defaults_vgp.yaml" | ||
- "{{ tpv_config_dir }}/environments_vgp.yaml" | ||
- "https://raw.githubusercontent.com/galaxyproject/tpv-shared-database/main/tools.yml" |
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.
If you cut regular releases we can create PRs for updates and test them out. That way we're getting timely updates without the surprise factor.
Co-authored-by: Nuwan Goonasekera <2070605+nuwang@users.noreply.github.com>
The reason for the drama with overriding is because of the loading order, causing the tpv shared db to override local config, instead of local config always overriding the shared db. This fixes that, and also uses the short url for the shared db. Just in case, I've added some extra tests for overriding default inheritance that simulates main pretty closely: galaxyproject/total-perspective-vortex#132
I've also made a PR restoring the original changes to the tpv shared db: galaxyproject/tpv-shared-database#63.