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

Improvements for dry-run cli #55

Open
natefoo opened this issue Nov 18, 2022 · 1 comment
Open

Improvements for dry-run cli #55

natefoo opened this issue Nov 18, 2022 · 1 comment
Labels
enhancement New feature or request

Comments

@natefoo
Copy link
Member

natefoo commented Nov 18, 2022

From the comments on #52 and #48:

  • We could avoid having to specify the Galaxy job conf by reading galaxy.yml and parsing the job conf out of there. However, adding this functionality directly to TPV (including automatically locating galaxy.yml) is reinventing a wheel (and a rather complicated and messy one) that is already done by Gravity and Galaxy itself. Ideally we would separate this out into a library that all 3 projects could then use.
  • The dry-run is not going to work for more complex custom rules because we are mocking Galaxy objects that won't have all of the attributes that a custom rule might access (e.g. object_store). We can look at a couple of things, including Galaxy's IntegrationTestCase and the minimal app that celery workers load.
  • There is currently no way to specify any tool param values or input sizes when performing a dry run. EDIT: --input-size added to Add a tpv dry-run subcommand #52.
@nuwang nuwang added the enhancement New feature or request label Nov 21, 2022
@nuwang
Copy link
Member

nuwang commented Dec 6, 2022

An additional enhancement from the original issue suggested by @mira-miracoli:

Add explanation for which destination was chosen and why. eg:

RESULT for .....
--------------------------------------------------------------------------------------
DESTINATION
final: condor_tpv
matched(with ranks): condor_tpv(1), singularity(2), interactive(3)
--------------------------------------------------------------------------------------
PARAMS
cores: 8 (min of tool: 10, user: 8, role: 9)
mem: ....
...

Additionally an explanation of how the rank was calculated in this specific case, would be helpful to debug errors in the configuration and check if it works as intentioned.

@nuwang nuwang mentioned this issue Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants