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

Add Docker caching #29

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Add Docker caching #29

merged 1 commit into from
Sep 19, 2024

Conversation

maxslarsson
Copy link
Contributor

@maxslarsson maxslarsson commented Sep 19, 2024

Fixes #31

Copy link
Contributor Author

maxslarsson commented Sep 19, 2024

.gitignore Outdated Show resolved Hide resolved
Copy link
Contributor

@ZachGarcia42 ZachGarcia42 left a comment

Choose a reason for hiding this comment

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

:)

Copy link
Contributor

@ZachGarcia42 ZachGarcia42 left a comment

Choose a reason for hiding this comment

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

Fix merge conflicts please

Base automatically changed from 09-16-dockerize_fill_station_binary to main September 19, 2024 01:04
Copy link
Contributor

@ZachGarcia42 ZachGarcia42 left a comment

Choose a reason for hiding this comment

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

Are we able to just rerun the workflow to see a speed increase from caching?

@ZachGarcia42 reviewed this PR from Slack with Graphite

@maxslarsson
Copy link
Contributor Author

maxslarsson commented Sep 19, 2024

Yea we can, but I don't think it'll have a massive speed increase unfortunately because of how Docker works. Basically it treats each line in the Dockerfile as a layer, so whenever the inputs for one line changes, it has to rebuild every line below. So whenever we change any code, it invalidates the COPY . . line, which means we have to rerun each line below which includes the bazel build line. That means I don't think it will get cached. Ideally, we'd be able to build the dependencies for our program on a seperate line before the COPY . ., so that that gets cached and the COPY . . is put after.

Do you know if there is a Bazel command to download and build the dependencies only?

Copy link
Contributor

ZachGarcia42 commented Sep 19, 2024

I don't think that's too feasible unfortunately. 13 minutes isn't terrible. If that's slower than we want, we can create a second docker file for the workflow and move back to uploading/downloading an artifact from the bazel tests. I think this is cleaner than copying some of the repo, doing a build step, copying the rest, and then building the actual target

@maxslarsson
Copy link
Contributor Author

Yea for sure, I think 13 minutes is fast enough for our purposes, at least for now. If not, we can go back in history and get your artifact solution again.

I think most of the time we test the code locally, and the Docker build is just to get onto the RPi

Copy link
Contributor

@ZachGarcia42 ZachGarcia42 left a comment

Choose a reason for hiding this comment

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

Thanks for this improvement!

@maxslarsson maxslarsson merged commit 2dc68f2 into main Sep 19, 2024
4 checks passed
@maxslarsson maxslarsson deleted the 09-18-add_docker_caching branch September 19, 2024 05:04
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.

Add Docker caching
2 participants