feat: Runner gRPC endpoint #446
Conversation
darunrs
force-pushed
the
runnerEndpoint
branch
from
7645a9d
to
94bea8d
Compare
|
Need to figure out that snyk error. I don't currently have access to view what's failing for some reason. Asking Ops. |
|
I added tests but its under the "Load Diff". I think its hidden by default since its a bit large. |
|
Also, regarding transitioning between old and new, I'm still working on ideas for that. Do you think I should include enabling the server in this PR or not? I felt like this was already a lot and maybe I can just release the implementation and tests, and separately enable the server with a good old to new transition logic in it. What do you think? |
Let's do it in a follow up PR to keep the work scoped. We could just create two entrypoints, and then have the main |
runner/src/service/runner-service.ts
Outdated
| return null; | ||
| } | ||
|
|
||
| function validateStartExecutorRequest (request: StartExecutorRequest__Output, streamHandlers: StreamHandlers): any | null { |
There was a problem hiding this comment.
Why do we need this manual validation? Shouldn't this be done by grpc and/or typescript?
There was a problem hiding this comment.
I thought the same but when I made the client and tested just calling one of the endpoints with missing fields, it went through. The callback I got was the error I had coded myself.
darunrs
force-pushed
the
runnerEndpoint
branch
from
fea7c8c
to
a2a4538
Compare
There was a problem hiding this comment.
Great job on this 👏🏼
Mostly looks good, left a few more minor comments. The validation still doesn't feel right, I feel like that should be implemented out of the box, can you spend some more time investigating please 🙏🏼 if we end up needing it feel free to merge 😄
|
I'll investigate validation some more! I agree it looks awkward. |
|
There are no required fields in Protocol Buffers. Their docs state you can have an optional, repeated, or map type. I wasn't able to find any resources that indicate it's available in proto3. Apparently proto2 had it though. |
|
I think running build before test in that way causes some problems in github. Most likely its the deleting of the dist folder, since that's where the snapshots are saved. Not really sure since tsc should regenerate them. But in any case, I have to keep it as codegen && test instead of build (codegen inside) && test. I'll commit this as is for now, and I'll improve it later if I need to. Same goes for validation. |
As part of implementing a new control plane to oversee QueryApi resources, Runner needed endpoints with which control plane can send commands. This PR adds code to create a new gRPC server with endpoints to start, stop, or list Runner executors. This provides the control plane the ability to fully control Runner, and removes the need for bilateral decision making, which was a problem of the previous design.
The changes will be changed further as needs become more concrete, closer to integration/release.