Skip to content

Proxy Registry: refactoring, Redis Client: add support for Proxy Registry #50834

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 7, 2025
Merged

Proxy Registry: refactoring, Redis Client: add support for Proxy Registry #50834

merged 3 commits into from
Nov 7, 2025
+622 −381

Conversation

@Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Nov 4, 2025 •
edited
Loading

Follows up on #50709
Fixes #50822

@Ladicek Ladicek requested review from cescoffier and ppalaga November 4, 2025 12:37
@Ladicek Ladicek changed the title Redis client proxy registry @Ladicek Redis Client: add support for Quarkus Proxy Registry Nov 4, 2025
@Ladicek Ladicek changed the title @Ladicek Redis Client: add support for Quarkus Proxy Registry Redis Client: add support for Quarkus Proxy Registry Nov 4, 2025
@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 4, 2025

@ppalaga Could you please also take a look? Especially at the 2nd commit, where I fix a few glaring issues. I'd personally do a bigger rewrite (e.g. I don't like how the recorder object and the bean are the same thing, or how Optional is used all around the place), but I decided to stay conservative :-)

@quarkus-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link

github-actions bot commented Nov 4, 2025 •
edited
Loading

🎊 PR Preview 66808d9 has been successfully built and deployed to https://quarkus-pr-main-50834-preview.surge.sh/version/main/guides/

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@quarkus-bot

This comment has been minimized.

Sign in to view
@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 4, 2025

I'm aware of the test failures and I'm looking for the best solution. It might involve a deeper refactoring of the Proxy Registry, which I originally wanted to avoid :-)

cescoffier
cescoffier previously requested changes Nov 4, 2025
View reviewed changes
Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

After a slightly deeper look, I agree with @Ladicek. We would need to change the proxy config.

ppalaga
ppalaga approved these changes Nov 5, 2025
View reviewed changes
...proxy-registry/runtime/src/main/java/io/quarkus/proxy/config/ProxyConfigurationRegistry.java Outdated
}
final Optional<NamedProxyConfig> namedProxyConfig = namedProxyConfigs.getOrDefault(plainName, Optional.empty());
if (namedProxyConfig == null) {
if (namedProxyConfig.isEmpty()) {
Copy link
Contributor

@ppalaga ppalaga Nov 5, 2025

Choose a reason for hiding this comment

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

Good catch, thanks @Ladicek!

@ppalaga
Copy link
Contributor

ppalaga commented Nov 5, 2025

I don't like how the recorder object and the bean are the same thing, or how Optional is used all around the place), but I decided to stay conservative :-)

@Ladicek please feel free to go ahead. This is the right time to do it, while not many extensions were not made aware of the proxy registry yet.

@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 6, 2025

OK, will do.

@Ladicek
Proxy Registry: big refactoring
2a1e6c1 
The public API is in the `io.quarkus.proxy` package and consists of:

- `ProxyType`: an enum of proxy types (HTTP, SOCKS4, SOCKS5)
- `ProxyConfiguration`: an interface that provides access to configured proxy
- `ProxyConfigurationRegistry`: an interface that allows looking up a named
  proxy configuration, optimized for the most common usage

The `io.quarkus.proxy.runtime` package contains implementation, which is:

- `ProxyConfigurationRecorder`: Quarkus recorder that provides a `Supplier`
  that is used in 2 ways: a proxy registry carrier stored in a build item,
  and a recorder proxy for a `ProxyConfigurationRegistry` synthetic bean
- `ProxyConfigurationRegistryImpl`: implementation of `ProxyConfigurationRegistry`
  that is created by the recorder

The config interface is now in the `io.quarkus.proxy.runtime.config`, because
the interface itself is not a public API.

The `deployment` module gains a `ProxyRegistryBuildItem` that can be consumed
by build steps that need to invoke recorder methods with proxy configuration.
The `ProxyRegistryProcessor` hasn't changed much (except of the name).

Finally, this commit adds a few basic unit tests to the Proxy Registry extension.
@Ladicek
RESTEasy Reactive: adjust the Proxy Registry usage after recent refac…
311e134 
…toring
@Ladicek
Redis Client: add support for the Proxy Registry
29a907f
@Ladicek Ladicek force-pushed the redis-client-proxy-registry branch from 0532750 to 29a907f Compare November 6, 2025 15:55
@Ladicek Ladicek changed the title Redis Client: add support for Quarkus Proxy Registry Proxy Registry: refactoring, Redis Client: add support for Proxy Registry Nov 6, 2025
@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 6, 2025

I refactored the Proxy Registry extension, so I'd like to ask @ppalaga and @geoand for review. I think the most important parts are:

  1. The ProxyConfigurationRegistry and ProxyConfiguration interfaces
  2. The fact that proxy configuration is now resolved eagerly. That is mostly interesting in case a credentials provider name is configured, because that means during startup, all credentials providers are looked up and the credentials are obtained from them. I'm not entirely sure that's best, but there's decent chance the users of Proxy Registry are initialized eagerly as well, so I figured it might not be a big deal, but I might easily be wrong on that. I'd love to hear your thoughts about that.

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 6, 2025

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 29a907f.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 6, 2025 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 29a907f.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ MicroProfile TCKs Tests

📦 tcks/microprofile-lra

❌ org.eclipse.microprofile.lra.tck.TckRecoveryTests.testCancelWhenParticipantIsUnavailable - History

  • Expecting the metric Compensated callback was called Expected: a value equal to or greater than <1> but: <0> was less than <1> - java.lang.AssertionError
java.lang.AssertionError: 
Expecting the metric Compensated callback was called
Expected: a value equal to or greater than <1>
     but: <0> was less than <1>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.eclipse.microprofile.lra.tck.TckRecoveryTests.assertMetricCallbackCalled(TckRecoveryTests.java:210)
	at org.eclipse.microprofile.lra.tck.TckRecoveryTests.testCancelWhenParticipantIsUnavailable(TckRecoveryTests.java:195)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)

ppalaga
ppalaga approved these changes Nov 6, 2025
View reviewed changes
Copy link
Contributor

@ppalaga ppalaga left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @Ladicek!

@geoand geoand dismissed cescoffier’s stale review November 7, 2025 05:40

Issue addressed

@geoand geoand merged commit f2f695b into quarkusio:main Nov 7, 2025
45 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.30 - main milestone Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@geoand geoand geoand approved these changes

@cescoffier cescoffier cescoffier left review comments

+1 more reviewer

@ppalaga ppalaga ppalaga approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/redis area/rest area/rest-client triage/flaky-test

Projects

None yet

Milestone

3.30 - main

Development

Successfully merging this pull request may close these issues.

Make quarkus-redis-client aware of quarkus-proxy-registry

4 participants

@Ladicek @ppalaga @cescoffier @geoand