Stop relying on builder image for CI. #3674
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fulmicoton
force-pushed
the
issue/rest-test-in-ci
branch
13 times, most recently
from
c46fa66
to
dadb81d
Compare
fulmicoton
commented
Jul 24, 2023
| @@ -16,7 +16,7 @@ env: | |||
| RUST_BACKTRACE: 1 | |||
| RUSTFLAGS: -Dwarnings | |||
| RUSTDOCFLAGS: -Dwarnings -Arustdoc::private_intra_doc_links | |||
| TEST_DATABASE_URL: postgres://quickwit-dev:quickwit-dev@postgres:5432/quickwit-metastore-dev | |||
| TEST_DATABASE_URL: postgres://quickwit-dev:quickwit-dev@localhost:5432/quickwit-metastore-dev | |||
There was a problem hiding this comment.
Without the extra container nesting, postgres is actually accessible as localhost.
The previous CI was strangely relying on workflows to split the CI and have it run in parallel. A lot of the work was made several times: e.g. installing rust stable + nightly, installing packages, starting our builder image etc. Overall it was done x6 times. It was strongly overkill considering some of the steps run in (<1s). After this refactoring we run in two "jobs" with a different set of steps. - Lint runs clippy + all of the smaller jobs (cargo fmt, cargo deny, checking the license) - Test runs the unit test. In addition, we stop relying on the builder image and the associated confusion. For instance, the rust stable installed in the CI workflow was actually not used. The quickwit-builder image one was taken because of the rust-toolchain.toml. After the PR, we run on stock ubuntu-latest directly. We then rely on rustup to install the right version of cargo/rust as define in the rust-toolchain. Updating the rust version does not require any extra changes. cargo deny / cargo nextest need to be build unfortunately. That step is however cached.
fulmicoton
force-pushed
the
issue/rest-test-in-ci
branch
from
dadb81d
to
03df32e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.