- Notifications
You must be signed in to change notification settings - Fork3k
Proxy registry integration for GraphQL clients#51286
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Thanks for your pull request! Your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
This comment has been minimized.
This comment has been minimized.
737174e to185ff61Compare| // use the proxy configuration registry | ||
| resolveProxyConfiguration(quarkusConfig) | ||
| .ifPresent(proxyConfiguration -> { | ||
| transformed.setProxyHost(proxyConfiguration.host()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, some properties of the proxy configuration aren't supported on the SmallRye side yet, so they are ignored. I will fix that later too, but it will need a SmallRye-side change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Alternatively, we could document somewhere which options are supported in this extension.
You could also warn on the console if unsupported optional settings are present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok I've added a warning ifnon-proxy-hosts is set and an error iftype != http
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
github-actionsbot commentedNov 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
🎊 PR Previewd2bb1ce has been successfully built and deployed tohttps://quarkus-pr-main-51286-preview.surge.sh/version/main/guides/
|
ppalaga left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks great, thanks@jmartisk!
| // use the proxy configuration registry | ||
| resolveProxyConfiguration(quarkusConfig) | ||
| .ifPresent(proxyConfiguration -> { | ||
| transformed.setProxyHost(proxyConfiguration.host()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Alternatively, we could document somewhere which options are supported in this extension.
You could also warn on the console if unsupported optional settings are present.
185ff61 to43bb59fComparequarkus-botbot commentedNov 28, 2025 • edited by github-actionsbot
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by github-actionsbot
Uh oh!
There was an error while loading.Please reload this page.
Status for workflow |
Status for workflow |
| }); | ||
| if (proxyConfiguration.type() !=ProxyType.HTTP) { | ||
| thrownewConfigurationException( | ||
| "Only HTTP proxy type is supported by the GraphQL client extension at the moment."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Would it be clear from the stack trace or elsewhere, which specific config key is incompatible with this extension? We could perhaps also suggest some corrective action. How about
"Quarkus SmallRye GraphQL extension currently supports only proxy type 'HTTP'. You may either change the value of 'quarkus.proxy"+ (quarkusConfig.proxyConfigurationName().isEmpty() ? "" : ("." + quarkusConfig.proxyConfigurationName().get)) +".type' to 'HTTP', or you can create a new proxy configuration having 'quarkus.proxy.\"some-name\".type = HTTP' and set 'quarkus.graphql.proxy-configuration-name = some-name'.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Similarly, the config key could be named also in the above Non-proxy hosts message.
No description provided.