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.

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

Update documentation for handling proxies in native-mode #45004

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Update documentation for handling proxies in native-mode #45004

merged 1 commit into from
Dec 16, 2024

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Dec 9, 2024

  • Merge "Managing Proxy Classes" sections.
  • Remove outdated mention to -H:DynamicProxyConfigurationResources

Quarkus no longer uses -H:DynamicProxyConfigurationResources for
dynamic proxies. Instead it generates the corresponding necessary
metadata in META-INF/native-image/proxy-config.json

gastaldi
gastaldi approved these changes Dec 9, 2024
View reviewed changes
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

LGTM

gsmet
gsmet requested changes Dec 9, 2024
View reviewed changes
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 spotted a small typo.

I'm not sure I'm completely sold about merging the two sections given how this document has been built (and given these are two very different scopes). But happy to be proven wrong if you have a strong opinion about it :)

docs/src/main/asciidoc/writing-native-applications-tips.adoc Outdated
@@ -650,11 +644,15 @@ public class S3Processor {
}
----

Using such a construct means that a `-H:DynamicProxyConfigurationResources` option will automatically be added to the `native-image` command line.
This will allow Quarkus to generate the necessary configurtion for handling the proxy class.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This will allow Quarkus to generate the necessary configurtion for handling the proxy class.
This will allow Quarkus to generate the necessary configuration for handling the proxy class.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch! You have eagle eyes :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🦅 fixed

docs/src/main/asciidoc/writing-native-applications-tips.adoc Outdated
Using such a construct means that a `-H:DynamicProxyConfigurationResources` option will automatically be added to the `native-image` command line.
This will allow Quarkus to generate the necessary configurtion for handling the proxy class.
Alternatively, you may create a `proxy-config.json` file under the `src/main/resources/META-INF/native-image/<group-id>/<artifact-id>` folder.
For more information about the format of this file, see the link:https://www.graalvm.org/{graalvm-docs-version}/reference-manual/native-image/metadata/#dynamic-proxy-metadata-in-json[Dynamic Proxy Metadata in JSON] documentation.
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, you don't need the link: when it's a URL starting with https?://

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just followed the existing pattern (it's easier than visiting the docs :) )
Removed

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, it was more for the future, no worries.

@zakkak
Update documentation for handling proxies in native-mode
c09ade9 
* Merge "Managing Proxy Classes" sections.
* Remove outdated mention to `-H:DynamicProxyConfigurationResources`

Quarkus no longer uses `-H:DynamicProxyConfigurationResources` for
dynamic proxies. Instead it generates the corresponding necessary
metadata in `META-INF/native-image/proxy-config.json`
@zakkak zakkak force-pushed the 2024-12-09-native-proxy-doc-update branch from 733b819 to c09ade9 Compare December 9, 2024 13:39
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2024 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Dec 9, 2024

Status for workflow Quarkus Documentation CI

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

✅ 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.

@gsmet gsmet merged commit 7261453 into quarkusio:main Dec 16, 2024
5 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Dec 16, 2024
@zakkak zakkak deleted the 2024-12-09-native-proxy-doc-update branch December 17, 2024 08:45
@zakkak
Copy link
Contributor Author

zakkak commented Dec 17, 2024

I'm not sure I'm completely sold about merging the two sections given how this document has been built (and given these are two very different scopes). But happy to be proven wrong if you have a strong opinion about it :)

Oh, I never commented on that but it looks like you went ahead with it anyway.
I believe avoiding duplication in documentation (especially in the same page) is the right thing to do, but we could include a reference if you think a mention to Proxy registration is necessary in place of the old block.

@gsmet
Copy link
Member

gsmet commented Dec 17, 2024

Yeah, I understand the thing about duplication but in this case, things are a bit rather different between extensions and apps.

If you can add a pointer, that would be perfect.

zakkak added a commit to zakkak/quarkus that referenced this pull request Dec 17, 2024
@zakkak
Improve documentation for handling proxies in native-mode
036cc7f 
Follow up to quarkusio#45004
@zakkak zakkak mentioned this pull request Dec 17, 2024
Merged
@gsmet gsmet modified the milestones: 3.18 - main, 3.17.5 Dec 17, 2024
gsmet pushed a commit to gsmet/quarkus that referenced this pull request Dec 18, 2024
@zakkak @gsmet
Improve documentation for handling proxies in native-mode
44f09de 
Follow up to quarkusio#45004

(cherry picked from commit 036cc7f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gastaldi gastaldi gastaldi approved these changes

@gsmet gsmet gsmet approved these changes

Assignees
No one assigned
Labels
area/documentation
Projects
None yet
Milestone
3.17.5
Development

Successfully merging this pull request may close these issues.
3 participants
@zakkak @gsmet @gastaldi