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

Stabilize CI pipelines #5108

Merged
merged 22 commits into from
Sep 9, 2024
Merged

Stabilize CI pipelines #5108

merged 22 commits into from
Sep 9, 2024

Conversation

carlesarnal
Copy link
Member

@carlesarnal carlesarnal commented Sep 2, 2024

This PR introduces the following changes to address a few transient failures in CI:

  • No more parallel build. The code generation phase is not thread safe and this was causing some build to fail. We no longer build the separe modules in parallel.
  • Add a few required retries due to the nature of the kafkasql storage. All the tests are being executed in a clustered way, and this was causing a few eventual consistency issues. Added a few retries to address this problem.
  • Applied a few minor improvements like not using the install goal to speed up the full process.

Before merging, this PR has to be squashed to prevent polluting the history.

@carlesarnal carlesarnal changed the title Use server side apply to not pollute the logs Stabilize CI pipelines Sep 2, 2024
@carlesarnal carlesarnal force-pushed the stabilize-ci branch 2 times, most recently from 6f39359 to 4484a8d Compare September 2, 2024 14:00
@EricWittmann
Copy link
Member

Interesting changes - can you explain the reasoning for them? I'm curious. :)

@@ -59,7 +59,7 @@ jobs:
run: mvn -N io.takari:maven:wrapper -Dmaven=3.8.2

- name: Build Application
run: ./mvnw -T 1.5C clean install --no-transfer-progress -Pprod -DskipTests=true -Dmaven.javadoc.skip=true -Dmaven.wagon.httpconnectionManager.maxTotal=30 -Dmaven.wagon.http.retryHandler.count=5
Copy link
Member Author

Choose a reason for hiding this comment

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

You'll see a bunch of this. The application was being built in parallel, meaning that multiple modules were being built at the same time. Code generation invocation was not thread-safe, leading to a bunch of transient errors "related" to Kiota (quoted since the errors aren't caused by Kiota, just happening during that phase).

@@ -352,9 +352,9 @@ jobs:
echo "Starting Registry App (In Memory)"
docker run -it -p 8181:8080 -e apicurio.rest.deletion.artifact.enabled=true -d ttl.sh/${{ github.sha }}/apicurio/apicurio-registry:1d
cd registry-v2
./mvnw -T 1.5C -Pintegration-tests clean install -DskipTests=true -DskipUiBuild=true -Dmaven.javadoc.skip=true
Copy link
Member Author

Choose a reason for hiding this comment

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

Also changing from install to package since it makes very little sense to install things in the local repository here.

@carlesarnal carlesarnal force-pushed the stabilize-ci branch 3 times, most recently from 1bc0231 to 8fd0373 Compare September 4, 2024 13:07
@carlesarnal carlesarnal force-pushed the stabilize-ci branch 2 times, most recently from 90e8ac7 to 14cdfd5 Compare September 5, 2024 07:30
@carlesarnal carlesarnal force-pushed the stabilize-ci branch 3 times, most recently from 27ed393 to adf6f08 Compare September 5, 2024 17:40
@carlesarnal carlesarnal force-pushed the stabilize-ci branch 2 times, most recently from 0eec49a to d82b759 Compare September 9, 2024 14:01
@carlesarnal carlesarnal marked this pull request as ready for review September 9, 2024 15:18
@carlesarnal carlesarnal merged commit e62676e into Apicurio:main Sep 9, 2024
19 checks passed
@carlesarnal carlesarnal deleted the stabilize-ci branch September 9, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants