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

refactor: update Dockerfile for IFTPipeline.jl #174

Open
wants to merge 49 commits into
base: jghrefactor/A-gather-julia-package-in-new-directory
Choose a base branch
from

Conversation

hollandjg
Copy link
Collaborator

@hollandjg hollandjg commented Oct 26, 2024

Update Dockerfile to:

  • use the local version of the repo, rather than cloning from the origin repository first (helps testing)
  • set up the Conda environment more easily
  • run all the tests
  • provide an ENTRYPOINT so you can call docker run -it brownccv/icefloetracker-julia landmask and it'll automatically start up the CLI for the IFTPipeline.

Other changes required:

  • Move example pipeline config file into IFTPipeline.jl/tests/test_input directory
  • Provide a Julia project and script to set up the Conda environment on any machine
  • Update the GitHub actions to use the new Conda environment setup
  • Update the GitHub actions to build the new version of the Dockerfile
  • Update SOIT to use 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

…n environment and only needs to be run once"

This reverts commit 43f4187.
@hollandjg hollandjg force-pushed the jghrefactor/A-gather-julia-package-in-new-directory branch from 934c537 to 6a0c594 Compare October 28, 2024 17:58
@hollandjg hollandjg changed the title refactor: update icefloetracker dockerfile refactor: update Dockerfile for IFTPipeline.jl Oct 28, 2024
@hollandjg hollandjg requested review from tdivoll, cpaniaguam and mirestrepo and removed request for tdivoll November 4, 2024 16:04
@hollandjg hollandjg marked this pull request as ready for review November 4, 2024 21:20
Copy link
Collaborator

@tdivoll tdivoll left a 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()'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to confirm that line 15 indeed gives us python 3.11 and IFT 0.60:
image

Copy link
Member

@cpaniaguam cpaniaguam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some observations, thanks!

Copy link
Member

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()

Copy link
Member

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()'
Copy link
Collaborator Author

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.

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.

3 participants