Modify and Update CI Testing #95
Conversation
connoramoreno
changed the title
.github/workflows/docker_publish.yml
Outdated
| - name: populate environment | ||
| run: | | ||
| . /opt/etc/bashrc | ||
| conda activate parastell_env |
There was a problem hiding this comment.
Do we need this line since it is also in the bashrc? (I'm the one who put it both places, but was going to come back and clean it up)
There was a problem hiding this comment.
I guess we should decide whether we would rather have the command executed via . /opt/etc/bashrc or run explicitly as here. What all is the bashrc used for? It seems like it's just to activate the conda environment
There was a problem hiding this comment.
I think the bashrc also sets some of the path variables. We build this on top of an image that already has some version of conda installed, and the path info is in the bashrc, so I think we need to keep some of it. Happy to leave the environment activation in the action though for clarity.
Dockerfile
Outdated
| RUN git clone https://github.com/svalinn/parastell.git && \ | ||
| cd parastell && \ | ||
| git checkout gh-action |
There was a problem hiding this comment.
We should probably change this to do a checkout in the GH action, and then COPY . parastell or something to copy it all into the current location. Then we always get the version associated with the current checkout.
There was a problem hiding this comment.
Does the action already do a checkout via actions/checkout@v4? I'm still learning GitHub Actions so this is slightly unclear to me
There was a problem hiding this comment.
I've removed this block from the Dockerfile and everything seems to work. Is that bad practice?
There was a problem hiding this comment.
No - this is OK! and probably better. We don't actually need to install parastell in the Dockerfile. The only reason to keep it may be to have a Dockerfile that can be used by others to make an image for Parastell?
There was a problem hiding this comment.
And if we did want that, we might want to have a stage that stops short of installing parastell that we use for the CI image we use here.
There was a problem hiding this comment.
But this Dockerfile is already tweaked so much to work in our particular way that it isn't a good basis for making a personal docker image anyway
There was a problem hiding this comment.
This looks good now @connoramoreno - thanks for figuring out what I did wrong
Modify and Update CI Testing
Rebases
gh_actionbranch from PR #84 ontooo_version, updates installation of CAD-to-DAGMC, and modifies use of GitHub secrets.