Skip to content
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

Update Dockerfile to focal and reorder steps #1158

Merged
merged 3 commits into from
Oct 2, 2023
Merged

Update Dockerfile to focal and reorder steps #1158

merged 3 commits into from
Oct 2, 2023

Conversation

jychp
Copy link
Collaborator

@jychp jychp commented Apr 14, 2023

This PR includes :

  • upgrade from bionic to focal (bionic is EOL this month)
  • reorder some step to allow layer cach for pip dependencies

Focal use python3.8, so everything should be OK.

Reordering steps allow pip dependencies install before all source code is copied (this is quite anoying for local tests ...)

@achantavy achantavy changed the title dockerfile improvment Update Dockerfile to focal and reorder steps Sep 20, 2023
@achantavy
Copy link
Contributor

It looks reasonable to me. @ryan-lane knows more a lot more about docker than me though - what do you think?

RUN pip install -e . && \
pip install -r test-requirements.txt

# Install cartography
COPY . /srv/cartography
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care to upgrade. Can you explain what this line is for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before :
copying source code was done before installing dependencies, so any change will trigger dependencies install

After :
first only requirements are copied, so the docker layer is cached
then files are copied, so change in files wont trigger a new dependencies install

@achantavy achantavy enabled auto-merge (squash) October 2, 2023 21:30
@achantavy achantavy merged commit 734e224 into cartography-cncf:master Oct 2, 2023
5 checks passed
chandanchowdhury pushed a commit to juju4/cartography that referenced this pull request Jun 26, 2024
This PR includes :
- upgrade from bionic to focal (bionic is EOL this month)
- reorder some step to allow layer cach for pip dependencies

Focal use python3.8, so everything should be OK.

Reordering steps allow pip dependencies install before all source code
is copied (this is quite anoying for local tests ...)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants