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

morph: Try all endpoints on client creation #2705

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

carpawell
Copy link
Member

Do not fail if the first one is unavailable but more endpoints are provided.

Copy link

codecov bot commented Dec 22, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (b82cb66) 28.79% compared to head (0d08b51) 28.78%.

Files Patch % Lines
pkg/morph/client/constructor.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2705      +/-   ##
==========================================
- Coverage   28.79%   28.78%   -0.01%     
==========================================
  Files         415      415              
  Lines       32371    32375       +4     
==========================================
- Hits         9321     9319       -2     
- Misses      22216    22221       +5     
- Partials      834      835       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/morph/client/constructor.go Show resolved Hide resolved
pkg/morph/client/constructor.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -6,6 +6,7 @@ Changelog for NeoFS Node
### Added

### Fixed
- Morph client dies when the first endpoint is unavailable even if there are more endpoints to try (#2703)
Copy link
Contributor

Choose a reason for hiding this comment

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

can be confusing: is it about what was fixed or after changes? from now / no more help

and Morph client -> NeoFS chain connection

Copy link
Member Author

Choose a reason for hiding this comment

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

is it about what was fixed or after changes?

it always bothers me when i fill Fixed. i came to a mentioning a problem itself (so every point here describes not expected things)

from now / no more help

i can understand you but not sure we want to see hundreds of from now it this file. mb we need to find some common approach or try to paraphrase it. @roman-khimov

and Morph client -> NeoFS chain connection

this is a common chain client

Copy link
Member

Choose a reason for hiding this comment

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

fixed smth, like "RPC client failure when..."

Copy link
Member Author

@carpawell carpawell Dec 28, 2023

Choose a reason for hiding this comment

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

agree, fixed. but still think that we need to have some common behavior pattern here. a headache every time I need to expand this section even if I really want it to be clear

still think "... failure ..." can be understood differently. is it a new expected failure or we have fixed some unexpected failure?

@carpawell carpawell force-pushed the fix/morph-creation-with-multiple-endpoits branch from 4e9e3e8 to 8a9a185 Compare December 25, 2023 12:49
Do not fail if the first one is unavailable but more endpoints are provided.
Closes #2703.

Signed-off-by: Pavel Karpy <[email protected]>
@carpawell carpawell force-pushed the fix/morph-creation-with-multiple-endpoits branch from 8a9a185 to 0d08b51 Compare December 28, 2023 12:24
@cthulhu-rider cthulhu-rider merged commit f4fd505 into master Dec 28, 2023
8 of 11 checks passed
@cthulhu-rider cthulhu-rider deleted the fix/morph-creation-with-multiple-endpoits branch December 28, 2023 16:27
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.

3 participants