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

[21.01] Refactor default mounting of $tmp_directory in containers #11654

Merged
merged 2 commits into from Apr 8, 2021

Conversation

innovate-invent
Copy link
Contributor

What did you do?

  • Refactored default mounting of $tmp_directory in containers
  • Reverted e9f2b3a

Why did you make this change?

e9f2b3a forced Galaxy to erroneously mount the host filesystem. In my case the path that $_GALAXY_JOB_TMP_DIR resolves to is in a docker volume. If multiple instances of Galaxy are running, they would mount the same path.

The issue is that $_GALAXY_JOB_TMP_DIR:$_GALAXY_JOB_TMP_DIR:rw assumes that $_GALAXY_JOB_TMP_DIR exists on the host.

The better solution is to add '$_GALAXY_JOB_TMP_DIR' to the $default value in docker_volumes runner spec.

The $tmp_directory variable can then resolve to self.job_info.tmp_directory or '$_GALAXY_JOB_TMP_DIR'.
I can then omit $tmp_directory from the docker_volumes config as the data volume where the job folder exists is already in the spec and the $_GALAXY_JOB_TMP_DIR path remains valid.

How to test the changes?

(select the most appropriate option; if the latter, provide steps for testing below)

  • 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]

For UI Components

  • I've included a screenshot of the changes

@github-actions github-actions bot added this to the 21.05 milestone Mar 17, 2021
@mvdbeek mvdbeek self-assigned this Apr 7, 2021
@mvdbeek mvdbeek merged commit c767adb into galaxyproject:release_21.01 Apr 8, 2021
@mvdbeek mvdbeek changed the title Refactor default mounting of $tmp_directory in containers [21.01] Refactor default mounting of $tmp_directory in containers Apr 8, 2021
@mvdbeek
Copy link
Member

mvdbeek commented Apr 8, 2021

Thanks, sorry, that took a while to test, but I think that's a good solution.

@innovate-invent innovate-invent deleted the revert_11448 branch April 8, 2021 18:15
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.

None yet

2 participants