Updated Dockerfile and added new run & build script - #213
Conversation
jtniehof
left a comment
jtniehof
left a comment
There was a problem hiding this comment.
My docker-fu is weak, so I'm going to rely on @pleasant-menlo to make sure nothing's gone awry.
Should we be removing run_imap_l3_processing_using_docker.sh and/or run_local_using_docker.sh? Dockerfile_run_local?
This reverts commit 4015b61.
…mmit (wrong file), then renamed it to existing file
jtniehof
left a comment
jtniehof
left a comment
There was a problem hiding this comment.
I'm going to go ahead and approve, although I'd like to have Menlo's eye on this, and I have a question about potentially updating the version file for a local docker build.
…or_using_docker.sh
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate issues affect version metadata, image architecture handling, and temporary-data availability.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (3)
What changed in this PR
Updates Docker image construction and adds a wrapper for running the IMAP L3 processor with amd64 support.
Changes:
- Reworks the Dockerfile into a multi-stage,
uv-based build. - Adds a focused
.dockerignore. - Replaces the legacy Docker wrapper with build-and-run logic.
| File | Summary |
|---|---|
run_imap_l3_processor_using_docker.sh |
Builds and runs the processor, but requires architecture validation, version generation, and temporary-data mounting fixes. |
run_imap_l3_processing_using_docker.sh |
Legacy wrapper replaced by the new workflow. |
Dockerfile |
Defines the multi-stage image; version metadata remains an unresolved issue. |
.dockerignore |
Restricts the Docker build context. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| docker run --platform=linux/amd64 --rm \ | ||
| -e IMAP_API_KEY \ | ||
| -e IMAP_DATA_DIR=/data \ | ||
| -v "$IMAP_DATA_DIR:/data" \ |
There was a problem hiding this comment.
temp_cdf_data already exists inside the image and is currently set up as scratch space for a single run by design. If people disagree, this can be changed.
…correct architecture
…r os/architecture (i.e., linux/amd64)
7320024
into
IMAP-Science-Operations-Center:main
|
Everything looks reasonable to us; the changes have been merged.
|
Overview
Closes #212.
File Changes
Dockerfile: documented so that it is easier to understand.run_imap_l3_processor_with_docker.sh: thin wrapper to launchimap_l3_processor.pyusing Docker. It will build the Docker image if it is not found.--playform=linux/amd64flag is passed to bothdocker buildanddocker run, ensuring that the executables inimap_l3_processing/glows/l3e/l3e_toolkitwork inside the image.