-
Notifications
You must be signed in to change notification settings - Fork 1
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
refactor: update Dockerfile for IFTPipeline.jl #174
base: jghrefactor/A-gather-julia-package-in-new-directory
Are you sure you want to change the base?
refactor: update Dockerfile for IFTPipeline.jl #174
Conversation
…nment and only needs to be run once
…n environment and only needs to be run once" This reverts commit 43f4187.
934c537
to
6a0c594
Compare
…to jghrefactor/A1-update-dockerfile
…to jghrefactor/A1-update-dockerfile
…/WilhelmusLab/ice-floe-tracker-pipeline into jghrefactor/A1-update-dockerfile
…to jghrefactor/A1-update-dockerfile
(moved to a different PR)
…/WilhelmusLab/ice-floe-tracker-pipeline into jghrefactor/A1-update-dockerfile
This reverts commit d1cb176.
…efactor/A1-update-dockerfile
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.
This is great, I think it just need Cylc updates? I'm happy to help with a little guidance to follow the new logic/entrypoint.
# IFT Pipeline package build | ||
#=========================================== | ||
COPY ./IFTPipeline.jl /opt/IFTPipeline.jl | ||
RUN julia --project="/opt/IFTPipeline.jl" -e 'using Pkg; Pkg.rm("IceFloeTracker"); Pkg.add(url="https://github.com/WilhelmusLab/IceFloeTracker.jl", rev="main"); Pkg.instantiate()' |
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.
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.
Some observations, thanks!
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.
It seems like this will never be imported into a different file or module. Why not drop the if check and collect the imports at the top? Something like
#!/usr/bin/env julia
using Conda, etc.
funciton main()
...
end
main()
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.
In view of this and #193 (comment), I think something like a makefile might be useful to simplify all of this.
# IFT Pipeline package build | ||
#=========================================== | ||
COPY ./IFTPipeline.jl /opt/IFTPipeline.jl | ||
RUN julia --project="/opt/IFTPipeline.jl" -e 'using Pkg; Pkg.rm("IceFloeTracker"); Pkg.add(url="https://github.com/WilhelmusLab/IceFloeTracker.jl", rev="main"); Pkg.instantiate()' |
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.
@hollandjg take another look at this.
Update Dockerfile to:
ENTRYPOINT
so you can calldocker run -it brownccv/icefloetracker-julia landmask
and it'll automatically start up the CLI for the IFTPipeline.Other changes required:
pipx
and inline script metadata rather than the python environment from the Julia package, because that environment isn't available for testing in the GitHub action where we test SOIT (where we don't set up Julia) – this is separated out into feat: update SOIT to use PIPX and an independent Dockerfile #175