-
Notifications
You must be signed in to change notification settings - Fork 267
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
Automatically build and push a docker image #696
Conversation
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.
@yunjunz @forrestfwilliams I've added some review comments with more details about some of the choices I've made here.
In particular, I'm interested in if you thinking this is the right approach for tagging the images.
Right now, I'm tagging any push to main latest
so if you docker pull ghcr.io/insarlab/mintpy:latest
you'll get whatever is on main
.
However, latest
in the docker world is typically used as "the image I want users to pull" which is generally "the last stable release", so I could also see instead tagging the last release latest
and any merge to main
as test
(or develop
or similar)
You can get a container build off this branch like: docker pull jhkennedy/mintpy:test You can test the docker container by running: docker run --rm -it jhkennedy/mintpy:test python tools/MintPy/tests/test_smallbaselineApp.py Or drop into the container to run the tests: docker run -it jhkennedy/mintpy:test
cd tools/MintPy/tests
python test_smallbaselineApp.py You can also run the jupyterlab instance by running: docker run --rm -p 8888:8888 -it jhkennedy/mintpy:test jupyter lab --no-browser --port=8888 --ip=0.0.0.0 --NotebookApp.token=mintpy (Might be worth making a jupyter launch script...) |
Thank you for the so long desired PR @jhkennedy!!! I feel that we are better off if splitting the "requirements + circle CI" related changes to another PR. For each of them, I would like to update the related documentation and links as well during the PR review to make sure that 1) code and doc are consistent and 2) we are all on the same page for the related instruction. What do you say? I have updated
We should update the version of I will take a closer look at the docker file and workflow related changes later this week. |
Having |
@yunjunz Agreed! I'll split those out into a different PR. Changes kinda cascaded on me.
Yes! I wanted to make sure the tagging pattern matched what you and @forrestfwilliams thought before investing in the docs.
That sounds great! I'll work on the conda-forge PR first, then split out the requirements into a seperate PR that can go before this one, and then we can all circle back to this one, giving @forrestfwilliams a little time to look it over. |
👍, @forrestfwilliams does that match your feeling as well? |
Hi @jhkennedy, yes I think it makes sense to split this into two separate PRs as @yunjunz suggested. As for the image tags, I like the idea of having the |
Okay, conda-forge/pyaps3-feedstock#2 is open, so the PyAPS v0.3.0 should be available soon. |
@yunjunz I'll have time to pick this up again tomorrow -- I've been slammed the last couple months |
Sounds great @jhkennedy, please take your time. I have been using pip-install for the development version of mintpy as you suggested, instead of path setup, and it feels great! With a few minor fixes that will be included in the next release, I believe the conda-installed version of mintpy will be functioning in its full capacity. |
I rebase this PR against the latest Regarding the documentation, I could think of the following:
A question I have been wondering regarding docker is: for users who want to use isce2 and mintpy together via docker, what would be your recommended way to proceed? |
Thanks @yunjunz I think I'll be able to work on this again next week some -- I'll work on addressing 1-4.
There's a lot of options -- easiest is create a new conda environment for isce2 when you jump into the container. It's also possible to build a docker image that combines an ISCE2 image and this image though I'd have to look at how the ISCE2 one is laid out. We could also build an ISCE2 env into this container... |
I like the idea of adding |
@yunjunz finally getting back to this -- I just forced push rebasing onto insarlab/MintPy@main so I can see where we're at and will work on the outstanding stuff now |
@yunjunz and @forrestfwilliams alright, I updated the documentation now as well, so I think this is ready for another review. I did push a couple container images to insarlab/mintpy in the GitHub Container Registry, so you can:
Note: By default the MintPy container images repository is set to private, so any MintPy maintainer can pull these as long as they log into ghcr.io with docker, but general users won't be able to until the repository is made public. @yunjunz if you want to make the container repository public, you (or another owner) will need to go to: You may also notice the container "packages" show up on the side of the GitHub repository now (if you have access to them): |
@yunjunz @forrestfwilliams I pushed some changes to make the Jupyter integration a little easier, and updates the docs on how to start the Jupyter server. As such, I pushed an updated image based on this PR: docker pull ghcr.io/insarlab/mintpy:v1.3.3-39-g5f718b2b I'll rebuild and push the Also, Codacy is complaining about two lines in the |
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.
Thank you @jhkennedy for finishing this exciting PR!!! Both the implementation and documentations look great. I have changed the container image (package) visibility to public, following your suggestion.
Testings, including tools/MintPy/tests/test_smallbaselineApp.py
, on my laptop work very well, except for one issue: when I started an interactive shell session as below, view.py
seems to work well (normal print-out messages), except that the plotting window is not popping up. Any idea?
docker run -it -v ~/data:/home/mambauser/data --name mintpy ghcr.io/insarlab/mintpy:latest
cd data/test/FernandinaSenDT128/mintpy
view.py velocity.h5
FYI, view.py velocity.h5 --nodisplay
also works (successfully plotted and saved the figure into a file). So this seems to be an interactive plotting window issue.
@yunjunz I'm not sure -- we're using a pretty minimal base image so it might be a missing |
@yunjunz the issue does come down to X.Org windows system issue, that is a bit annoying to work around and can induce some security risks. Further reading:
There's a couple tools that solve a lot of the issues that are more-or-less straightforward to use that set up X.Org windows system between the host and the container: But since we include Jupyter, that's the easiest/best path to plotting figures in the container. |
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.
Thank you very much for the useful information regarding GUI @jhkennedy. I agree that Jupyter is a good and easy path for us right now. More work on the GUI in a Docker can be done laterward in another PR, I have created an issue for it (#801).
As for this PR, it looks all great to me. Cheers!
The Github workflow failed with the following error, which seems to be related to some permission issue. @jhkennedy I have added you as a maintainer role. Any idea how we could resolve this? https://github.com/insarlab/MintPy/runs/7099087138?check_suite_focus=true#step:6:1603
|
Hmm, can you confirm that the I don't think I can look at the org/repo setting without admin or owner level permissions. |
@yunjunz I think I found the setting needed: Re-running the action now to check |
Great! |
This PR:
adds an GitHub Actions workflow for docker which:
main
main
and then pushes the images to the GitHub Container Registrylatest
v*
tag (e.g.,v1.3.1
) and then pushes the images to the GitHub Container Registry1.3.1
for av1.3.1
tag)updates the
Dockerfile
(again)git+https
jupyterlab
andipympl
in the environment by defafultupdates the environment/requirements files to use the conda-forge version of
pyaps3
andpysolid
instead of cloning or installing viagit+https
Reminders