Closed phlg closed 1 year ago
For a first step seems good to me we will see later if we should split certificates and repos for the moment let just move the code as it is.
For renv, the code seems to do something only if one variable at least is injected by onyxia that what i would have do too.
For the shebang, let's align to avoid some problems, https://www.baeldung.com/linux/bash-shebang-lines it seems that the existing shebang is more portable.
Thanks for the review, I fixed the shebang and marked the PR to be ready.
I think we should set repo before executing user scripts.
Linked to a couple of PRs on the helm charts :
This PR changes a couple changes a couple of things related to repository configuration :
onyxia-set-repositories.sh
file, which in turn is now called inonyxia-init.sh
. The main purpose for this was to ease "private" rebuilding of these images. As might be common for other users rebuilding their own custom images, we needed to run the repository configuration at build time, and not only runtime. Because of this, we managed a script ripped fromonyxia-init.sh
containing all the pertaining commands. By having a dedicated file "upstream", we can just call the file which will have been baked in the image already.⚠️ There are a fair number of points I'm not sure about though, please take them into account when reviewing :
onyxia-init.sh
:PATH_TO_CA_BUNDLE
andcertifi_ca.py
)renv.config.repos.override
) - I don't know if this should be done only when we set R_REPOSITORY and/or PACKAGE_MANAGER_URL, or all the time.onyxia-init.sh
is#!/usr/bin/env bash
, but I naively went for a "basic"#!/bin/bash
inonyxia-set-repositories.sh
. Is that fine, or should I align withonyxia-init.sh
? I'm not sure what this implies.onyxia-init.sh
was explicitly chmodded even though its containing directory was recursively chmodded just a few lines prior (see the first and last lines in this excerpt : https://github.com/InseeFrLab/images-datascience/blob/main/base/Dockerfile#L31-L77)