-
Notifications
You must be signed in to change notification settings - Fork 15
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
Adds bookworm into test images #339
Conversation
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why so many empty lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there were empty spaces in other files and I formed this one from others. Removed empty spaces for all files now
# install prequisities | ||
RUN apt-get update \ | ||
&& apt-get install -y lsb-release \ | ||
apt-utils \ | ||
vim \ | ||
wget \ | ||
curl \ | ||
gnupg2 \ | ||
software-properties-common \ | ||
libcurl4-openssl-dev \ | ||
libssl-dev | ||
|
||
|
||
# install Citus | ||
RUN apt-get update \ | ||
&& apt-get install -y --no-install-recommends \ | ||
ca-certificates \ | ||
curl \ | ||
&& curl -s https://install.citusdata.com/community/deb.sh | bash | ||
RUN apt-get install -y postgresql-${PG_MAJOR}-citus-${CITUS_MAJOR_VERSION}=${CITUS_VERSION}.citus-${FANCY} \ | ||
postgresql-$PG_MAJOR-hll=${HLL_VERSION} \ | ||
postgresql-$PG_MAJOR-topn=${TOPN_VERSION} \ | ||
&& rm -rf /var/lib/apt/lists/*12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we not merge these RUN
commands?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in sake of debugging purposes. If there is an error in a command, I have issues to find which statement causes this error.
Since this is not an image to publish, merging them will not give a benefit to us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is actually a benefit: the resulting docker image will be smaller. Every apt command that does not include rm -rf /var/lib/apt/lists/*
at the end will result in bigger docker images, and having to download bigger docker images result in longer CI run times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not like having continuous RUN commands in dockerfiles. However, I leave the decision to improve their performance to the author.
Debian bookworm is added to package tests in this PR