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

Fix proxy gw selection when a gw is down #268

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

fholzhauser
Copy link
Contributor

No description provided.

@fholzhauser fholzhauser requested a review from a team as a code owner November 24, 2020 18:34
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 78.363% when pulling 1373f24 on fix/proxy_gw_selection_gw_down into d4d6a42 on master.

@vkatsuba
Copy link
Contributor

Good catch! Maybe make sense add some test case for this fix?

@fholzhauser
Copy link
Contributor Author

This is a fairly trivial/safe fix and also needed with some urgency. Extending CT to cover this previously missed scenario is planned in a follow up PR.

@mgumz mgumz merged commit e606f62 into master Nov 25, 2020
@mgumz mgumz deleted the fix/proxy_gw_selection_gw_down branch November 25, 2020 07:29
Copy link
Contributor

@ebengt ebengt left a comment

Choose a reason for hiding this comment

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

Looks good

@RoadRunnr
Copy link
Member

This is a fairly trivial/safe fix and also needed with some urgency. Extending CT to cover this previously missed scenario is planned in a follow up PR.

The interesting thing is that this line was actually hit by the tests, but the catch all a few lines above hide the problem.

In fact adding a when is_record(Socket, socket) to the catch all would have made the unit tests fail.

@vkatsuba
Copy link
Contributor

The interesting thing is that this line was actually hit by the tests, but the catch all a few lines above hide the problem.

In fact adding a when is_record(Socket, socket) to the catch all would have made the unit tests fail.

Currently the checking of socket record already exist in choose_gw/4 however the test cases in proxy_lib_SUITE don't crashed by reason that in node_selection section provided one by one node. For raise an error like function_clause need add into config section node_selection two nodes where first node is invalid and next node is valid, using this configuration example CT in previous version can raise an error like function_clause for case if choose_gw/4 will be called as choose_gw(Next, NodeSelect, Socket, Version) instead of expected choose_gw(Next, NodeSelect, Version, Socket). PR #270 witch updating of CT was provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants