Skip to content

Use make cli-local in CI test suite to remove redundant docker #6531

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

Merged
merged 3 commits into from
Oct 22, 2024

Conversation

jimmygchen
Copy link
Member

Issue Addressed

I noticed that our cli job sometimes take quite a while and take a few docker pull retries before it completes. This is because it attempts to pull the 2.2 GB github-runner image to run the the cli script in a docker container.

I think we can safely replace the cli command to run with cargo directly instead of running it with a github-runner image, as the job runs in a container running the github-runner image, so this is a bit redundant and slow.

@jimmygchen jimmygchen added infra-ci ready-for-review The code is ready for review labels Oct 21, 2024
Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

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

Approved pending one smol nit sorry.
Tbh, I'm not sure why we weren't using make cli-local in CI in the first place 😅

Copy link
Member

@macladson macladson left a comment

Choose a reason for hiding this comment

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

Oh also, the title isn't really accurate anymore, I'll change it to something like "Use make cli-local in CI"

@macladson macladson changed the title Remove docker command from make cli Use make cli-local in CI test suite to remove redundant docker Oct 22, 2024
Co-authored-by: Mac L <[email protected]>
@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Oct 22, 2024
@jimmygchen
Copy link
Member Author

@mergify queue

Copy link

mergify bot commented Oct 22, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at ad229a6

mergify bot added a commit that referenced this pull request Oct 22, 2024
@mergify mergify bot merged commit ad229a6 into sigp:unstable Oct 22, 2024
29 checks passed
chong-he pushed a commit to chong-he/lighthouse that referenced this pull request Nov 26, 2024
…p#6531)

* Remove docker command from `make cli`.

* Run `cli-local` on CI.

* Update Makefile

Co-authored-by: Mac L <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra-ci ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants