-
Notifications
You must be signed in to change notification settings - Fork 179
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
[Access] Refactor Access RPC engines to support a single gRPC port #4217 #4411
[Access] Refactor Access RPC engines to support a single gRPC port #4217 #4411
Conversation
…pc and state_stream engines, refactored access_node_builder
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.
Overall, I really like this approach. Added a few comments on specifics, but you're on the right track.
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.
thanks for making all those changes. It's coming together.
Can you also add an integration test that verifies both continue to work when configured
configured on the AccessAPI
and ExecutionDataAPI
on the same port
@@ -612,8 +617,7 @@ func (builder *FlowAccessNodeBuilder) BuildExecutionDataRequester() *FlowAccessN | |||
node.RootChainID, | |||
builder.executionDataConfig.InitialBlockHeight, | |||
highestAvailableHeight, | |||
builder.apiRatelimits, | |||
builder.apiBurstlimits, | |||
builder.unsecureGrpcServer, |
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.
previously, this could be configured on a separate port. Please keep that functionality, but add support for configuring it on the same port.
Codecov Report
@@ Coverage Diff @@
## master #4411 +/- ##
==========================================
- Coverage 54.48% 53.66% -0.83%
==========================================
Files 914 750 -164
Lines 85356 72536 -12820
==========================================
- Hits 46505 38924 -7581
+ Misses 35262 30743 -4519
+ Partials 3589 2869 -720
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Some stylistic comments, otherwise good job!
bors merge |
4411: [Access] Refactor Access RPC engines to support a single gRPC port #4217 r=durkmurder a=UlyanaAndrukhiv #4217 ### Context This pull request * creates a new grpcserver module which includes a GrpcServer and a GrpcServerBuilder for it. GrpcServer defines a grpc server that starts once and uses in different Engines * refactored the Access RPC engine and state stream engine setup using the same grps server. Also small refactored Observer node's engine setup * refactored tests according to new changes Co-authored-by: UlyanaAndrukhiv <u.andrukhiv@gmail.com> Co-authored-by: Uliana Andrukhiv <u.andrukhiv@gmail.com>
Build failed: |
bors retry |
4411: [Access] Refactor Access RPC engines to support a single gRPC port #4217 r=durkmurder a=UlyanaAndrukhiv #4217 ### Context This pull request * creates a new grpcserver module which includes a GrpcServer and a GrpcServerBuilder for it. GrpcServer defines a grpc server that starts once and uses in different Engines * refactored the Access RPC engine and state stream engine setup using the same grps server. Also small refactored Observer node's engine setup * refactored tests according to new changes Co-authored-by: UlyanaAndrukhiv <u.andrukhiv@gmail.com> Co-authored-by: Uliana Andrukhiv <u.andrukhiv@gmail.com>
Build failed: |
bors retry |
4411: [Access] Refactor Access RPC engines to support a single gRPC port #4217 r=durkmurder a=UlyanaAndrukhiv #4217 ### Context This pull request * creates a new grpcserver module which includes a GrpcServer and a GrpcServerBuilder for it. GrpcServer defines a grpc server that starts once and uses in different Engines * refactored the Access RPC engine and state stream engine setup using the same grps server. Also small refactored Observer node's engine setup * refactored tests according to new changes Co-authored-by: UlyanaAndrukhiv <u.andrukhiv@gmail.com> Co-authored-by: Uliana Andrukhiv <u.andrukhiv@gmail.com>
Build failed: |
bors retry |
4411: [Access] Refactor Access RPC engines to support a single gRPC port #4217 r=durkmurder a=UlyanaAndrukhiv #4217 ### Context This pull request * creates a new grpcserver module which includes a GrpcServer and a GrpcServerBuilder for it. GrpcServer defines a grpc server that starts once and uses in different Engines * refactored the Access RPC engine and state stream engine setup using the same grps server. Also small refactored Observer node's engine setup * refactored tests according to new changes Co-authored-by: UlyanaAndrukhiv <u.andrukhiv@gmail.com> Co-authored-by: Uliana Andrukhiv <u.andrukhiv@gmail.com>
Build failed: |
#4217
Context
This pull request