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

Migrate Quarkus Vert.x HTTP extension config classes to @ConfigMapping #44115

Open
michalvavrik opened this issue Oct 26, 2024 · 4 comments
Open
Labels
area/config area/housekeeping Issue type for generalized tasks not related to bugs or enhancements area/vertx kind/enhancement New feature or request

Comments

@michalvavrik
Copy link
Member

michalvavrik commented Oct 26, 2024

Description

I have mentioned many extensions are migrating to @ConfigMapping. I am waiting for a moment when the Vert.x HTTP extension will be migrated because lately we talked once again with @sberyozkin about fluent API to setup Security #16728 and - this is just my opinion - interfaces would help because they establish contract. If someone changes field or adds a new field of a config class, we might forget to update builders. But if the @ConfigMapping interface changed, builders we could provide will simply fail to compile.

Now, I know that @radcortez already migrated this extension #35246 and then reverted it #35756, but since then, I have seen so many migrations, lately in core module #42114 that I don't understand why is this extension so special. We should have at least plan or tracker and this is why I am opening this issue.

Implementation ideas

I am happy to help, but I think Roberto has it covered. For users, we could provide update recipe, for extension writers, I think they can handle...

@michalvavrik michalvavrik added area/config area/housekeeping Issue type for generalized tasks not related to bugs or enhancements area/vertx labels Oct 26, 2024
Copy link

quarkus-bot bot commented Oct 26, 2024

/cc @radcortez (config)

@radcortez radcortez added the kind/enhancement New feature or request label Oct 26, 2024
@radcortez
Copy link
Member

Now, I know that @radcortez already migrated this extension #35246 and then reverted it #35756, but since then, I have seen so many migrations, lately in core module #42114 that I don't understand why is this extension so special. We should have at least plan or tracker and this is why I am opening this issue.

This is definitely not forgotten; I have a local branch with that work. We had to revert it at the time due to performance issues, but those problems were already fixed, so it should be safe to move.

What is holding this for now is that many extensions use the Vert.x http configuration directly to retrieve the port and a few other things so that it will affect many extensions. Extensions should not be using this, and we were discussing better alternatives for it:

So ideally, we wanted to introduce a better API to retrieve the port, instead of relying on the configuration, so extensions would only need to move once and not twice (from Root -> to new API, instead of from Root -> Mapping -> new API).

But I guess this is taking too long, so we might want to go the not-so-optimal route.

@michalvavrik
Copy link
Member Author

I have subscribed myself to all three PRs so that I get notified. Thanks for the links. I'll leave this issue opened so that there is a tracker for this migration (so that I get notified as well). Thank you

@michalvavrik
Copy link
Member Author

To be fair, I have loads of other issues I can work on, we are in no hurry, so please migrate at your own pace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config area/housekeeping Issue type for generalized tasks not related to bugs or enhancements area/vertx kind/enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants