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

fix remote resource check TPV rule and enable it #967

Conversation

sanjaysrikakulam
Copy link
Member

This rule now handles "None" case as well.

Question:
Should we fail the job when the remote resource the user selects (even unconsciously (the presence of old tags in DB and the user forgot about it, and they have not changed it to the currently existing ones)) does not exist or tackle it in TPV and let it go to default?

Copy link
Contributor

@kysrpex kysrpex left a comment

Choose a reason for hiding this comment

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

Should we fail the job when the remote resource the user selects (even unconsciously (the presence of old tags in DB and the user forgot about it, and they have not changed it to the currently existing ones)) does not exist or tackle it in TPV and let it go to default?

Yes, I think we should fail it. If the user expects the job to go to a specific destination and it turns out it goes somewhere else then they may go crazy.

Take this hypothetical example: I send my jobs to a Pulsar cluster where there are GPU machines with 128GB of RAM. The Pulsar cluster is decommissioned at some point (removed from the list of available destinations), and I send a job that needs 100GB of RAM. If we do not fail the job with the rule from this PR, the user will just get an error telling them that there was no destination available to run the job (because we do not have GPU machines with that much RAM on EU), but they will not know the underlying reason.

files/galaxy/tpv/tool_defaults.yml Outdated Show resolved Hide resolved
Co-authored-by: José Manuel Domínguez <[email protected]>
@sanjaysrikakulam sanjaysrikakulam merged commit cb97f36 into usegalaxy-eu:master Oct 24, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants