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

Containerized set metadata #17164

Draft
wants to merge 27 commits into
base: dev
Choose a base branch
from

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Dec 11, 2023

This is how it's enabled:


                "metadata_config": {
                    "containerize": True,
                    "engine": "docker",
                    "image": "galaxyproject/galaxy-job-execution",
                },

inside your environment (see integration test). The image is of course not published yet. docker build -f packages/job_execution/Dockerfile . -t galaxyproject/galaxy-job-execution to build the image if you want to play with something that is not the integration test.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@github-actions github-actions bot added this to the 23.2 milestone Dec 11, 2023
@mvdbeek mvdbeek requested a review from a team December 11, 2023 15:55
@mvdbeek mvdbeek force-pushed the containerized_set_metadata branch 3 times, most recently from c429262 to e8db1d1 Compare December 11, 2023 19:48
@mvdbeek
Copy link
Member Author

mvdbeek commented Dec 12, 2023

lagom.exceptions.TypeOnlyAvailableAsAwaitable: <no detail available> is specific to python 3.8 and doesn't happen on 3.11 ... guess the EdamDictType type variable is at fault.

natefoo added a commit to galaxyproject/usegalaxy-playbook that referenced this pull request Dec 13, 2023
@mvdbeek mvdbeek force-pushed the containerized_set_metadata branch from 96516d2 to 14889e8 Compare December 14, 2023 11:45
@bernt-matthias
Copy link
Contributor

Would it be a good idea to document this, e.g. in the job_conf.sample.yml or docs. Maybe with usecases, ie when this should be used.

Guess this works only (and makes sense) in containerized job destinations?

@mvdbeek mvdbeek marked this pull request as draft December 15, 2023 16:32
@mvdbeek
Copy link
Member Author

mvdbeek commented Dec 15, 2023

Sure, once this is working and we've agreed on the details.

@mvdbeek mvdbeek force-pushed the containerized_set_metadata branch 4 times, most recently from 809411a to 0f48828 Compare December 18, 2023 14:23
@mvdbeek mvdbeek modified the milestones: 23.2, 24.0 Dec 19, 2023
@mvdbeek mvdbeek force-pushed the containerized_set_metadata branch from 0d12e60 to 91734a2 Compare December 19, 2023 15:41
It's only used in API responses and the tool panels.
We surely don't want to load edam data when loading the registry while
setting metadata.
That happens when `$tool_directory` is part of the mount string and we
set `tool_directory` to None for the metadata container.
Boy is this silly. I really love that TS is smarter with this.
this used to be wrong if the app-wide default
`outputs_to_working_directory`
conflicts the destination's `outputs_to_working_directory`.
@mvdbeek mvdbeek force-pushed the containerized_set_metadata branch from 8890acb to 3333b7d Compare December 20, 2023 19:27
@mvdbeek mvdbeek force-pushed the containerized_set_metadata branch from 3333b7d to 57d7858 Compare December 20, 2023 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants