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

chore(docker): ensure that just up <backend> builds the image if necessary #8549

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

kszucs
Copy link
Member

@kszucs kszucs commented Mar 5, 2024

Noticed in #8541 after changing the flink dockerfile that the image hasn't been rebuilt.

Copy link
Contributor

github-actions bot commented Mar 5, 2024

ACTION NEEDED

Ibis follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message.

Please update your PR title and description to match the specification.

@kszucs kszucs changed the title dev(docker): ensure that just up <backend> builds the image if necessary chore(docker): ensure that just up <backend> builds the image if necessary Mar 5, 2024
@@ -98,7 +98,7 @@ download-data owner="ibis-project" repo="testing-data" rev="master":

# start backends using docker compose; no arguments starts all backends
up *backends:
docker compose up --wait {{ backends }}
docker compose up --build --wait {{ backends }}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this change is necessary. I never have this problem with any of the containers we build, they get built automatically if necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, nope. I'm wrong.

I don't really like this solution because it will rebuild images even if they don't need to be rebuilt, but I'm not sure what the alternative is.

Copy link
Member

Choose a reason for hiding this comment

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

There are only two images that we build so maybe it's not a huge problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

Docker layer cache will ensure that these are not actually rebuilt from scratch, it does nothing if the layer/image already exists.

Copy link
Member

Choose a reason for hiding this comment

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

I see a build of the image every time no matter what when running just up postgres.

Copy link
Member

Choose a reason for hiding this comment

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

I think it will still show the build no matter what, but the various steps should be prefixed with CACHED

Copy link
Member

Choose a reason for hiding this comment

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

e.g.

 => [postgres 1/2] FROM docker.io/postgis/postgis:15-3.3-alpine@sha256:4738bee  0.0s
 => CACHED [postgres 2/2] RUN apk add --no-cache postgresql15-plpython3         0.0s

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok!

@cpcloud cpcloud added this to the 9.0 milestone Mar 5, 2024
@cpcloud cpcloud added the developer-tools Tools related to ibis development label Mar 5, 2024
@cpcloud cpcloud merged commit 5c121df into ibis-project:main Mar 5, 2024
76 of 78 checks passed
@kszucs kszucs deleted the just-up-build branch March 5, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-tools Tools related to ibis development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants