Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
18.09 update #464
18.09 update #464
Changes from 17 commits
7d834ec
9998785
beb8440
1be3274
2efd525
c194c51
1481493
5bd1ba5
9efc04c
2312f4f
b533b20
d456e53
716b859
8f42276
57dcde9
933b13a
05be0b7
209b057
edbef97
b50f1d1
95a5d7c
33d03bc
52f92d4
e938b86
7e53dc0
5516786
72866ed
c21cfcc
7ec499e
27ee4ab
2c0be52
40b74ac
7beba07
bb3083d
83e2f24
1cf3215
76185aa
e5b586f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, I must have been testing a few things on 19.01 when I pushed this... sorry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been thinking for a while on incorporating
https://github.com/bgruening/docker-galaxy-stable/blob/dev/compose/dumpsql.sh#L9-L36
inside the build-orchestration-images script, right after the init container creation (using that init container created). Then this whole file would be generated on the fly when building the containers, based on the version that it was used of Galaxy for those containers (so fairly failure-proof). Would you consider adding that to your PR @abretaud ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that would be cool. I'm not sure how to do it properly though: do you mean that at the end of build-orchestration-images we should:
Not sure I'll have time to write this now, maybe it could be another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, don't worry, I'll PR it soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although beyond this PR, it would be really great if things like slurm dependencies could be installable via ansible and then that could be enabled or disabled via injection of arguments on build time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pcm32 are you using the pre-build images or are you building the images on your own again? Or what is your preferred use-case? We could also define those libs via an ENV and create different web containers during build-time - not sure this is a use-case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe ourselves building different flavours might be too much trouble to maintain, but I would at least leave the flexibility for the advanced user who knows what is doing. I like the model of the labels that we have, where to add the support for certain things, you need to add that label (ie. k8s,cvmfs, etc). So I would extend that, as man power allows, to other sections (like telegraf, slumr, condor, etc) that are optional requirements depending on how you are using this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bgruening I'm probably still building my own because I need to point to a version of ansible galaxy extras that includes this: galaxyproject/ansible-galaxy-extras#213
On my setup, it fails because uswgi process I think points to that galaxy.yml but that file doesn't exists at that point... the test not passed seems to be unrelated to the logic added on that PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bgruening in your compose setup works without my PR ansible-galaxy-extras due to https://github.com/bgruening/docker-galaxy-stable/blob/dev/compose/docker-compose.yml#L327 but the current PR (464) is actually removing that line, so the compose setup will need the solution proposed in https://github.com/bgruening/docker-galaxy-stable/blob/dev/compose/docker-compose.yml#L327 I think....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the mount of galaxy.yml because when you launch docker-compose up, the file doesn't exist yet, so Docker decides to create an empty directory named "galaxy.yml". This makes it impossible later to copy the good galaxy.yml file during startup. So galaxy fails to start because it can't read galaxy.yml.
Mounting a non existing file will never work as it will be considered a directory by docker