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

[UNDERTOW-2254] Included the HttpServerExchange in the HostSelector. #1448

Closed
wants to merge 2 commits into from
Closed

[UNDERTOW-2254] Included the HttpServerExchange in the HostSelector. #1448

wants to merge 2 commits into from

Conversation

jeham
Copy link

@jeham jeham commented Mar 23, 2023 •
edited by fl4via

This allows for advanced routing based on eg. header values or attributes.
Jira: https://issues.redhat.com/browse/UNDERTOW-2254

@fl4via fl4via added the new feature/API change New feature to be introduced or a change to the API (non suitable to minor releases) label Mar 25, 2023
@fl4via fl4via changed the title Included the HttpServerExchange in the HostSelector. [UNDERTOW-2254] Included the HttpServerExchange in the HostSelector. Mar 25, 2023
fl4via
fl4via requested changes Mar 25, 2023
View reviewed changes
Copy link
Member

@fl4via fl4via left a comment •
edited

Choose a reason for hiding this comment

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

@jeham I'm approving this for Undertow 2.4.0.Final (because of the changes in signatures).
However, we will want to keep the old HostSelector .selectHost methods and mark them as deprecated with forRemoval set to true.
I'll also ask you to add the [UNDERTOW-2254] prefix to the commit message.
Thank you for your PR!

@fl4via fl4via added waiting PR update Awaiting PR update(s) from contributor before merging waiting CI check Ready to be merged but waiting for CI check labels Mar 25, 2023
@fl4via
Copy link
Member

fl4via commented Mar 25, 2023

Also, please rebase it because the test failures we are seeing are supposed to have been fixed in master (UNDERTOW-2252), so hopefully after rebase they will be gone.

@jeham
[UNDERTOW-2254] Included the HttpServerExchange in the HostSelector.
e259cd8 
 This allows for advanced routing based on eg. header values or attributes.
 Jira: https://issues.redhat.com/browse/UNDERTOW-2254
@jeham
Copy link
Author

jeham commented Mar 28, 2023

Thanks for the feedback @fl4via !
I updated the code and the commit message.
I tried to rebase but got conflicts in all POM files though.

@jeham jeham requested a review from fl4via March 28, 2023 21:44
@fl4via
Copy link
Member

fl4via commented Apr 4, 2023

Thanks for the feedback @fl4via ! I updated the code and the commit message. I tried to rebase but got conflicts in all POM files though.

@jeham do you mind if I do the rebase then? I can pull a rebased version and kick off CI.
There is one thing that is calling my attention, though, and when I first reviewed this I didn't take notice. The PR should be done against master.
Do you need this feature specifically for 2.2.x?

@jeham
Copy link
Author

jeham commented Apr 12, 2023

Thanks for the reply @fl4via - I'll create a new PR from master.

@jeham jeham closed this Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fl4via fl4via Awaiting requested review from fl4via

Assignees
No one assigned
Labels
new feature/API change New feature to be introduced or a change to the API (non suitable to minor releases) waiting CI check Ready to be merged but waiting for CI check waiting PR update Awaiting PR update(s) from contributor before merging
Projects
None yet
Milestone
No milestone
2 participants
@jeham @fl4via