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

Updated dockerfile & added release workflow #11

Merged
merged 6 commits into from
Aug 18, 2022
Merged

Conversation

boris-martin
Copy link
Contributor

Changed the dockerfile to install FEniCSx from Ubuntu packages, then install the adapter. I also added a Github Action to push an image on Docker Hub. It would work once 1) A token is set on the repository 2) The PR #1 is merged. Before pushing, I added a rudimentary test: push if and only if python3 -c "import fenicsxprecice" runs without error.

Currently, the adapter is installed in regular mode. It might be interesting to add the -e flag so that this image can be used efficiently for debugging. Opinions welcome!

@arvedes
Copy link
Contributor

arvedes commented Aug 18, 2022

Currently, the adapter is installed in regular mode. It might be interesting to add the -e flag so that this image can be used efficiently for debugging. Opinions welcome!

I would rather not install the adapter in editable mode, because I think modifying the adapter stored inside the container is not best practice.

My development workflow (on windows) is to mount a local copy of the repository into the Docker container and install it in editable mode (either manual or using an additional entrypoint.sh script):

docker run -it --rm -v $PWD/:/home/fenicsx-adapter image-name

In this way I am independent of the container (which I remove after closing with the -rm) and have all changes stored locally.

@boris-martin
Copy link
Contributor Author

So when you start the container you remove the pre-built with pip uninstall then install the mounted one? Or is it enough to install the new one because pip will override the pre-installed version?
I like your workflow (I lost some time setting up remote editing through Docker, adding Git credential into new containers etc) so it makes sense, but what's the purpose of pre-installing anything then? Wouldn't you want just a container with FEniCSx and no adapter at this point? Shall we just keep a pre-installed version only for automated testing ?

@arvedes
Copy link
Contributor

arvedes commented Aug 18, 2022

I used a container without adapter: https://github.com/nemocrys/dolfinx-openfoam/blob/main/docker/Dockerfile . I'm not sure about the behavior with pre-installed adapter, I'd just try it out.

The version with the pre-installed adapter is definitely useful for testing. Maybe, some people will use it as a reference for installation (at least I do that sometimes ;) ) or directly work with the container in their own projects.

@boris-martin
Copy link
Contributor Author

I just tested, and installing local version automatically triggers a (clean) uninstall of the pre-installed version.
So let's keep the non-editable install, then the user will be free to override it easily.

@boris-martin boris-martin merged commit 5d70f02 into develop Aug 18, 2022
@boris-martin boris-martin deleted the update_dockerfile branch August 18, 2022 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide Dockerfile as basis for testing in this repository
3 participants