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

feat: Create Rust gRPC client for Runner #479

Closed
wants to merge 9 commits into from
Closed

feat: Create Rust gRPC client for Runner #479

wants to merge 9 commits into from

Conversation

darunrs
Copy link
Collaborator

@darunrs darunrs commented Jan 3, 2024

A Rust gRPC Client for Runner is necessary to allow Coordinator V2 to control executors. Coordinator V2 will be responsible for keeping executors up to date and handle failures in executors (e.g. worker crashes).

Coordinator V2 will go in another folder. Therefore, most of the code here needs to be migrated once Coordinator V2 is ready.

@darunrs darunrs self-assigned this Jan 3, 2024
@darunrs darunrs changed the title Create Rust GRPC client for Runner #468 feat: Create Rust gRPC client for Runner Jan 3, 2024
@darunrs darunrs marked this pull request as ready for review January 4, 2024 00:00
@darunrs darunrs requested a review from a team as a code owner January 4, 2024 00:00
@darunrs
Copy link
Collaborator Author

darunrs commented Jan 5, 2024

Had offline discussion with Morgan about the below:

Hey Morgan. Something I noticed just now while thinking over the rust client. I remembered that the docker file structure is a bit different than local so I tried to docker compose build and it actually broke on two fronts: protoc not installed and proto not found. The protoc part is an easy fix: update the Dockerfile. The proto not found I think is a bit tougher. I'd need to copy it into the docker, but that's not doable since the build context is indexer/, which doesn't include runner. So, I have these options for proceeding, I believe:

  • Don't copy anything, add a .ok() to forcibly suppress the tonic_build errors (if any), and figure out the file later. (What is currently in PR now)
  • Expand build context for coordinator to include runner, copy the file in, and adjust Dockerfile accordingly.
  • Copy the file directly into coordinator for now and implement a better solution later.

As a result of the discussions, we agreed that the amoutn of code that depends on his #444 PR is too high. So, he can integrate this work into his PR instead and resolve any integration problems that might exist. I will test out approach 2 today since we know 1+3 theoretically works already.

@morgsmccauley
Copy link
Collaborator

Duplicates #491

@morgsmccauley morgsmccauley deleted the rustClient branch January 8, 2024 02:03
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.

2 participants