Closed quarkus-super-heroes-bot closed 2 months ago
I may have isolated a change that can lead to this. I am working on a fix.
The build is still failing:
The build is still failing:
The build is still failing:
The build is still failing:
The build is still failing:
@ozangunalp any further word you might have on this?
@edeandrea here is the state of my analysis:
I don't want to blindly revert the change that breaks this code – which itself fixes a bug in the Kafka connector.
I am trying to understand how the code from SuperStats
worked in the first place.
Independent of the change, I don't think it was right to use mutiny context to store the message. (+ ack it before returning to the stream).
It stored the message in the context, then did some processing on that message. 2 different aggregations. 1 aggregation gets sent off to its own channel. After that, it continues processing and aggregating it differently, then returns it to a different channel. Both channels are in memory channels.
I couldn't figure out a way without using the context to hang onto the original message so that it could be ack'ed after both aggregations had completed.
Plus all the OpenTelemetry stuff is mixed in there too (capturing the parent span off the message, creating new child spans, closing then).
I'd love to remove all that otel complexity, but last time I tried then the spans didn't carry over the message properly. That was a few Quarkus versions ago though.
The build is still failing:
@edeandrea #195 is a temporary fix for this issue. I am working on a fix in Reactive Messaging, which may take more time.
Thank you @ozangunalp I merged it. Will see if this turns green tomorrow!
Thank you very much for your help!
Build fixed:
Unfortunately, the build failed:
The build is still failing:
@geoand I started looking at this most recent failure, which I can reproduce on my local machine.
For some reason when failsafe is running the ITs, the Quarkus app gets restarted multiple times. 3 times to be exact:
This restarting is what is causing the tests to fail. The ITs have an ordered sequence of tests that do things (i.e. one test creates data that the next test assumes is there). The fact that the app is restarting is killing the state of the app and causing test expectations to fail.
Any thoughts as to why this might be happening? Its only a single IT test class using @QuarkusIntegrationTest
.
The build is still failing:
@edeandrea https://github.com/quarkusio/quarkus/pull/29752 fixes the restart issue. If you include that fix, then you'll see the ITs failing with:
I'll pull this in locally and re-test to try to narrow down what the issue could be. Something in Quarkus changed, just have to figure out what.
The build is still failing:
Build fixed:
Unfortunately, the build failed:
@geoand /@gsmet this is due to the switch on Quarkus main to use the Jakarta namespace. This also means this job will continue to fail each and every day until Quarkus 3 is released and the superheroes has been upgraded to it.
We have a couple of options:
Personally I'd prefer option 1.
Are there any options I didn't think of?
See an updated setup-and-test
script: https://github.com/quarkusio/quarkus-ecosystem-ci/blob/f4d102f6dafdd4f2b72c8ef4b6b3ce59067a4862/setup-and-test#L11
How come it's not getting used then? It's pulling the Quarkus ecosystem ci....
https://github.com/quarkusio/quarkus-super-heroes/blob/main/.github/workflows/quarkus-snapshot.yaml
I'll let you answer that one by looking at the .github
directory of your repo and / or the CI run logs 😉
It's pulling from 'main' on quarkus-ecosystem-ci....
That contains the script which checks out the Quarkus repo. What am I missing?
@geoand I think I know what the problem is
https://github.com/quarkusio/quarkus-ecosystem-ci/blob/main/setup-and-test#L12
if [[ "$(mvn help:evaluate -Dexpression=quarkus.version -q -DforceStdout -f ../../current-repo/pom.xml| cut -d. -f1)" = "2" ]]; then
this assumes the root of the project has a pom.xml
at the root which declares the Quarkus version. In this case of the superheroes, this is not true. The superheroes has child projects, but is not itself multi-module.
Could we tweak the setup-and-test
script a little bit to maybe check if there is a get-quarkus-version
script in the project's repo, kind of like you do for this
# check the test script
if [ -f .github/quarkus-ecosystem-test ]; then
echo "Test script found"
else
echo "Test script not found - using default from quarkus-ecosystem-ci"
cp ../ecosystem-ci/quarkus-ecosystem-test .github/quarkus-ecosystem-test
fi
and let the script return the version to use?
I could do a PR for that if you're ok with the approach.
I wonder if it might be easier to go down a level and try to find a pom
Thats what I"m thinking too, but the setup-and-test
script would have to be injected with a path to follow - maybe via env var?
I don't think a generic setup-and-test
script should try to determine inside the project which pom to use. I think it would be better to let the project inject that info into the script.
Yeah, that's probably the right thing to do
So for example, in quarkusio/quarkus-super-heroes/.github/workflows/quarkus-snapshot.yaml
:
- name: Setup and Run Tests
run: ./ecosystem-ci/setup-and-test
env:
ECOSYSTEM_CI_TOKEN: ${{ secrets.ECOSYSTEM_CI_TOKEN }}
JAVA_17_PATH: ${{ env.JAVA_17_PATH }}
QUARKUS_VERSION_PATH: rest-fights
Then in quarkusio/quarkus-ecosystem-ci/setup-and-test
line 12 we would append the QUARKUS_VERSION_PATH
if it existed to find pom.xml
.
If you're ok with this approach I'll go ahead and do a PR.
also, the superheroes uses quarkus.platform.version
, not quarkus.version
in its pom.xml
, so we'd need to override/inject that into the script as well
👍🏼
@geoand see quarkusio/quarkus-ecosystem-ci#119
This should resolve itself tomorrow.
Build fixed:
Unfortunately, the build failed:
This failure is my fault. It is a side effect from quarkusio/quarkus-super-heroes#283. I will fix it today.
Build fixed:
Unfortunately, the build failed:
I feel like this is just some weirdness on the CI machine. Kafka Dev Services never was able to start a Kafka container. I'll let it go until tomorrow and see if it resolves itself.
Build fixed:
Unfortunately, the build failed:
Looks like the build of quarkus main itself failed. We'll see if this fixes itself tomorrow.
Build fixed:
Unfortunately, the build failed:
This issue will be open and closed dependent on the state of https://github.com/quarkusio/quarkus-super-heroes building against Quarkus main snapshot.
If you have interest in being notified of this subscribe to the issue.
Closing #23425 in favor of this one.