-
Notifications
You must be signed in to change notification settings - Fork 5
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
Bug fixes and API client experience enhancements #28
base: master
Are you sure you want to change the base?
Conversation
} | ||
catch (Exception ex) | ||
{ | ||
if(ex is ClusterException) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reformat the code. There should be an .editorconfig on the repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While your .editorconfig is in the solution, it isn't catching the formatting issue. I assume you mean the space after "if".?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, correct (the space is missing after if).
Sorry for the late response here.
await _requestDelegatingClient.Delegate(context.Request, context.Response, leaderAddress, localPort: context.Connection.LocalPort); | ||
return; | ||
context.Response.StatusCode = StatusCodes.Status503ServiceUnavailable; | ||
context.Response.ContentType = "application/json"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return; | ||
context.Response.StatusCode = StatusCodes.Status503ServiceUnavailable; | ||
context.Response.ContentType = "application/json"; | ||
string payload = JsonSerializer.Serialize(new { ex.Message, ExceptionType = ex.GetType().Name }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var
is the preferred code style
context.Response.ContentType = "application/json"; | ||
string payload = JsonSerializer.Serialize(new { ex.Message, ExceptionType = ex.GetType().Name }); | ||
context.Response.ContentLength = payload.Length; | ||
await context.Response.WriteAsync(payload); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering of you forgot top add return.
Just clarifying it we truly want to rethrow when the response is already provided here.
|
||
response.StatusCode = (int)delegatedResponse.StatusCode; | ||
if(delegatedResponse.StatusCode == System.Net.HttpStatusCode.NoContent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reformat please
Hey @Ephron-WL, Please add a signed-off statement to every commit. Thanks for your contribution. |
…ce enhancements. Idle state bug fix
@Ephron-WL let me know if something is unclear, or if you waiting on me here. Just checking. |
I was having problem with the analyzer and requested clarification regarding formatting (first comment you made the PR).
From: Tomasz Maruszak ***@***.***>
Sent: Monday, January 27, 2025 1:32 PM
To: zarusz/SlimCluster ***@***.***>
Cc: William Lucking ***@***.***>; Mention ***@***.***>
Subject: Re: [zarusz/SlimCluster] Bug fixes and API client experience enhancements (PR #28)
@Ephron-WL<https://github.com/Ephron-WL> let me know if something is unclear, or if you waiting on me here. Just checking.
—
Reply to this email directly, view it on GitHub<#28 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AKMJDOKQRSIR4O4M47LAFVD2M2QTRAVCNFSM6AAAAABVIOLXUOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJWHEZTQMRSHA>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
Bugs:
API Client Experience:
In cases were nodes fail and in cases where nodes are recreated by Kubernetes, calls to Increment might return null and HTTP status 204. This may occur because of one of four types of exceptions that may be thrown when requests are being received during state synchronization. The below changes were made to attempt to notify the client of the reason the API call failed so that the client can 1) understand the issue and 2) handle the issue.