Initial ngen-cal calibration worker image#303
Conversation
Adding "service" config to Docker build-time config file for building the ngen-calibration worker image.
Updating Dockerfile for ngen-calibration to support specifying a tag for the base ngen image to be used, if something other than "latest" is required.
aaraney
left a comment
aaraney
left a comment
There was a problem hiding this comment.
Looks good and is okay to merge, I think there are a few improvements that could be made but im not sure they are worth addressing just yet. I noted these areas in the comments.
| #REPO_URL: ${NGEN_REPO_URL?No NGen repo url configured} | ||
| NGEN_CAL_BRANCH: ${NGEN_CAL_BRANCH:-master} | ||
| NGEN_CAL_COMMIT: ${NGEN_CAL_COMMIT} | ||
| #TROUTE_REPO_URL: ${TROUTE_REPO_URL?No t-route repo url configured} |
There was a problem hiding this comment.
Since these are presumedly coming from the ngen image, I think we can just remove them for now? In the future we will need to revisit this when we support parallel calibration / calibration with routing. We will almost want some kind of build matrix for ngen images and calibration images to cover all the realistic ngen build configurations.
There was a problem hiding this comment.
We will almost want some kind of build matrix for ngen images and calibration images to cover all the realistic ngen build configurations.
Indeed, we are going to need to expand our image variant support. I expected it is reasonably likely that we will change things in the future in a way that will need these to still be here.
| # Change permissions for entrypoint and make sure dataset volume mount parent directories exists | ||
| RUN chmod +x ${WORKDIR}/entrypoint.sh \ | ||
| && for d in ${DATASET_DIRECTORIES}; do mkdir -p /dmod/datasets/${d}; done \ | ||
| && for d in noah-owp-modular topmodel cfe sloth 'evapotranspiration/evapotranspiration'; do \ |
There was a problem hiding this comment.
We may want to make this configuration by specifying these as a build time ARG. Thoughts?
There was a problem hiding this comment.
Not totally sure I follow, but I think it's something that can wait until the inevitable future revisions to this.
2 participants
Initial Docker image and build configuration for worker node in Nextgen calibration jobs using the ngen-cal utility.
Note that this PR partially replaces #240, in that it is composed of changes extracted from that PR for easier reviewing and merging.