-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Comments
/cc @radcortez (config) |
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. |
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 |
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. |
@radcortez, what's the status on this? I'm concerned about the API breaking change it can introduce (the Vert.x HTTP config class may be (are) used in other extensions. So, it would be a great time to do it now, between 2 LTS. |
there is #45274 opened that provides more info; I'm watching it because we are waiting for this, we plan to draft security fluent API as soon as it is done. |
The plan is to move it, but we will keep pieces of the old config API (deprecated) to prevent extensions from breaking (like we did with the DevServices config). |
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...
The text was updated successfully, but these errors were encountered: