Add Container Patch URLS at build time #227
Conversation
|
Not really sure if I did everything correctly. Sorry if there is any of the commit process wrong. I am of course open to any feedback or changes needed to get this merged. |
|
As a user I really enjoy this change, as I use my containers pre-built having the hability of using the patch_urls during my setup is awesome. Can't wait to have this merged. Tested here and worked as a charm! |
|
Hi @OrangeDrangon if this PR is ready, can you mark it as Ready for Review so we can review it? |
|
@felddy I am not sure how to proceed with the error of |
Adds apply_patches.sh to keep the Dockerfile from growing too complex. Can be used in `entrypoint.sh` as well I imagine. Changes the optional build step to write a `status.txt` out. Later use that file to know whether or not foundry was installed. I was unable to find a way to detect this built into to Docker and multistage builds do not persist environment variables across them. I am very open to other options. Then read back the file to determine whether or not to run the Container Patch URLs then remove it.. Container Patch URLs must be ran in the second stage because they may rely on the `/data` directory existing and what not. Running in the second step guarantees the same runtime environment they normally expect.
|
|
||
| RUN \ | ||
| if [[ $(cat status.txt) = "installed" ]]; then \ | ||
| source ./apply_patches.sh; \ |
There was a problem hiding this comment.
We may want to remove the ./ from here too, just to be consistent.
There was a problem hiding this comment.
Please also rebase your change as it's outdated with target branch as per This branch is out-of-date with the base branch
🗣 Description
Adds apply_patches.sh to keep the Dockerfile from growing too complex.
Can be used in
entrypoint.shas well I imagine.Changes the optional build step to write a
status.txtout. Later use that file toknow whether or not foundry was installed. I was unable to
find a way to detect this built into to Docker and multistage builds do
not persist environment variables across them. I am very open to other options.
Then read back the file to determine whether or not to run the Container
Patch URLs then remove it..
Container Patch URLs must be ran in the second stage because they may
rely on the
/datadirectory existing and what not. Running in thesecond step guarantees the same runtime environment they normally expect.
💭 Motivation and Context
See Issue #220
🧪 Testing
Built an image with various permutations of having and not having patch urls and credentials. Build ran successfully in all cases.
✅ Checklist
the changes in this PR.