search

galaxyproject / galaxy-helm

Minimal setup required to run Galaxy under Kubernetes
MIT License
41 stars 38 forks source link

Missing converters directory on jobs containers #425

Closed pcm32 closed 1 year ago

pcm32 commented 1 year ago

I came across the case of a tool (Query tabular using sqlite) that when given a CSV as input, it tried to convert the file into tabular before running, but failed with:

python: can't open file '/galaxy/server/lib/galaxy/datatypes/converters/tabular_csv.py': [Errno 2] No such file or directory

I'm not sure of an easy way out of this. Perhaps we might need to copy the converters or datatypes to the shared filesystem and configure galaxy to consume those from a different path? Or perhaps this metadata conversion step should run using the galaxy container itself? I'm guessing it is running that step on the tool's container. The whole job got handled a bit weird, in the sense that it didn't show as failed but as paused (saying that the input file was in error - sure, the CSV wasn't converted as desired into tabular).

pcm32 commented 1 year ago

This can be reproduced, in a slightly different manner (job shows up as failed), by using the converter functionality in the dataset view.

almahmoud commented 1 year ago

This used to be handled in the mapper script here: https://github.com/galaxyproject/galaxy-helm/blob/3bc6b61fe72401c60256e190fe2566c9b0f9d238/galaxy/files/rules/k8s_container_mapper.py#L92 specifically came up for csv_to_tabular but the fix was universal to all converters back in the day (https://github.com/galaxyproject/galaxy-helm/pull/348). I guess that was lost with the switch to the new rule mapper, so an equivalent rule probably needs to be added to map all converters to the base galaxy container. I am not familiar with the new rule mapper to easily do it, but hopefully that helps. @nuwang might be able to help figure out how to best add it to the new config

nuwang commented 1 year ago

I had completely forgotten about this @almahmoud. That should have got ported over during the conversion to TPV, but I had accidentally dropped it. Have made a PR with a fix, but would be great to get it tested @pcm32

pcm32 commented 1 year ago

Thanks for the fix! Does this require the newer tpv? We haven't migrated yet. To understand how far I would be of testing this.

nuwang commented 1 year ago

This should work on any version of TPV.

ksuderman commented 1 year ago

Whoops, I fat fingered the issue number in #436 and auto-closed the wrong issue. I am re-opening this until we can confirm the problem is actually fixed.

nuwang commented 1 year ago

Fixed in: https://github.com/galaxyproject/galaxy-helm/pull/426