added useLocalAuthorityResolver() to ServerBootstrap and AsyncServerBootStrap classes#556
added useLocalAuthorityResolver() to ServerBootstrap and AsyncServerBootStrap classes#556cdw8j wants to merge 1 commit intoapache:masterfrom cdw8j:feature/hc5-serverbootstrap-localauthority
Conversation
ok2c
force-pushed
the
master
branch
4 times, most recently
from
8fb1d8e to
bf7d524
Compare
|
Hello @garydgregory, seems this request is in deadlock, so let me explain the background of my request. I migrated a project from httpcore4 to httpcore5 just recently and when it didn't work, I had to dive into the httpcore5 sources to understand what was going wrong and that I need to create a custom RequestRouter with LOCAL_AUTHORITY_RESOLVER scheme to get the same behavior as before with httpcore4 just using simple ServerBootstrap methods. I might be the last on earth migrating an ancient httpcore4 application, but in this scenario and I guess also in others where the host name shall be irrelevant (and which is the reason why the LOCAL_AUTHORITY_RESOLVER scheme exists in httpcore5), it is quite a pain IMO to build the custom RequestRouter in the application itself, especially because this isn't really very well documented. But in the end, it's not my decision, so if you decide it's not worth the API extension, then just reject the pull request and nobody will get hurt either. |
|
@cdw8j Please address my comments if you want this pull request merged. |
| } | ||
|
|
||
| /** | ||
| * Create {@link RequestRouter} with LOCAL_AUTHORITY_RESOLVER (default: IGNORE_PORT_AUTHORITY_RESOLVER). |
There was a problem hiding this comment.
@cdw8j Please do not mention LOCAL_AUTHORITY_RESOLVER and IGNORE_PORT_AUTHORITY_RESOLVER. This is an implementation details and it should be mentioned in javadocs
| * | ||
| * @since 5.4 | ||
| */ | ||
| public final ServerBootstrap useLocalAuthorityResolver(final boolean localAuthorityResolver) { |
There was a problem hiding this comment.
@cdw8j Please use a better name for the method. #routeToLocalAuthority for example.
|
@ok2c Re-thinking this whole thing after it has been idling a bit in my head, I wonder if it wouldn't actually make more sense to implement setter methods in the What do you think? Should I create a separate pull request for this option and you decide which is preferrable? |
@cdw8j I personally like this idea much better. If you do it fast enough there is a a good chance it will make it into the coming 5.4-alpha1 release |
|
@ok2c not too much of a challenge :) see pull request #565 |
|
Superseded by #565 |
3 participants
(Follow-up to pull request #555 which I messed up while trying to rebase it from 5.3.x to master.)
I have added a convenience method to the ServerBootstrap and AsyncServerBootstrap classes that configures the RequestRouter to use the LOCAL_AUTHORITY_RESOLVER scheme. While it is possible to create an HttpServer with such a custom RequestRouter within the application, this results in really messy and complicated code, which can easily be spared with this very simple convenience method.