-
Notifications
You must be signed in to change notification settings - Fork 446
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
Replace JwtSecurityToken
with JsonWebToken
#7177
Conversation
@@ -10,7 +10,7 @@ namespace Nethermind.Sockets | |||
public interface IWebSocketsModule | |||
{ | |||
string Name { get; } | |||
ISocketsClient CreateClient(WebSocket webSocket, string client, HttpContext context); | |||
Task<ISocketsClient> CreateClient(WebSocket webSocket, string client, HttpContext context); |
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.
I would use valuetask as we won't need the task later
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.
Or no task at all?
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.
One of the interface implementations added an await call. Thats why it was added
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.
ah and it is the main one
@@ -10,7 +10,7 @@ namespace Nethermind.Sockets | |||
public interface IWebSocketsModule | |||
{ | |||
string Name { get; } | |||
ISocketsClient CreateClient(WebSocket webSocket, string client, HttpContext context); | |||
Task<ISocketsClient> CreateClient(WebSocket webSocket, string client, HttpContext context); |
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.
Or no task at all?
Changes
JwtSecurityToken
with the newJsonWebToken
that's up to 30% faster, as Microsoft claimsJwtAuthentication.Authenticate()
method async because of the underlying handler async API, which in turn caused chained async changes in several placesTypes of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?