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

Avoid double registration of providers #44052

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

radcortez
Copy link
Member

@radcortez radcortez commented Oct 23, 2024

When useBuiltinProviders from RESTEasy is true, the availableProviders will be automatically discovered by RESTEasy. If availableProviders are also included, RESTEasy will warn about double regisration.

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Oct 24, 2024

@cdsap The failure reported above look a bit concerning ^

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I think this makes things a lot more confusing. I'm not saying the new logic is wrong, just that it's very hard to follow.

We need to make sure the purpose of the sets is understandable and the new code is mixing them all to make it work for what you want to achieve and we completely lost the initial semantic.

I can take a stab at it if you don't feel like it.

@radcortez
Copy link
Member Author

I think this makes things a lot more confusing. I'm not saying the new logic is wrong, just that it's very hard to follow.

We need to make sure the purpose of the sets is understandable and the new code is mixing them all to make it work for what you want to achieve and we completely lost the initial semantic.

I can take a stab at it if you don't feel like it.

I agree. I just wanted to minimize the changes. Let me rework this properly.

@cdsap
Copy link
Contributor

cdsap commented Oct 24, 2024

Hi @gsmet,
after analyzing one of the failing tests, we have for the last 28 days:

  • 1 Flaky execution
  • 2 Failing executions

The flaky execution is caused by a 503 Service Unavailable when building the docker image on:

ERROR: failed to solve: registry.access.redhat.com/ubi8/openjdk-21:1.18: failed to resolve source metadata for 
registry.access.redhat.com/ubi8/openjdk-21:1.18: unexpected status from HEAD request to 
https://registry.access.redhat.com/v2/ubi8/openjdk-21/manifests/1.18: 503 Service Unavailable

Similarly, the failing executions are related to a 502 Bad Gateway when requesting registry.access.redhat.com/ubi8/openjdk-21:1.18:

ERROR: failed to solve: registry.access.redhat.com/ubi8/openjdk-21:1.18: failed to resolve source metadata for registry.access.redhat.com/ubi8/openjdk-21:1.18: unexpected status from HEAD request to 
https://registry.access.redhat.com/v2/ubi8/openjdk-21/manifests/1.18: 502 Bad Gateway

What are your thoughts on this? Is the service degradation a temporary issue, or should I avoid building the Docker images during these integration tests?

@radcortez radcortez requested a review from gsmet November 7, 2024 21:17

This comment has been minimized.

@gsmet gsmet force-pushed the resteasy-warnings branch from 374a502 to f26bd1d Compare November 25, 2024 15:59
@gsmet gsmet force-pushed the resteasy-warnings branch from f26bd1d to 0af8ae9 Compare November 25, 2024 15:59
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I rebased and fixed a small typo, let's merge when CI is green.

@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Nov 25, 2024
Copy link

quarkus-bot bot commented Nov 25, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 0af8ae9.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
MicroProfile TCKs Tests Verify Failures Logs Raw logs 🔍

You can consult the Develocity build scans.

Failures

⚙️ MicroProfile TCKs Tests #

- Failing: tcks/microprofile-opentelemetry 

📦 tcks/microprofile-opentelemetry

Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.0:test (tracing) on project quarkus-tck-microprofile-opentelemetry: There are test failures.

Please refer to /home/runner/work/quarkus/quarkus/tcks/microprofile-opentelemetry/target/surefire-reports-tracing for the individual test results.
Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.

@gsmet gsmet merged commit e77760f into quarkusio:main Nov 26, 2024
47 of 48 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Nov 26, 2024
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants