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

Define AI GRPC service to support using AI to generate cells #573

Merged
merged 5 commits into from
May 15, 2024

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented May 14, 2024

We'd like to use AI to generate cells. In particular, we'd like to support using Foyle to add AI capabilities.

The initial plan is to use a gRPC service to allow RunMe to communicate with Foyle (or potentially other AI services). This proto needs to define that gRPC service.

@jlewi
Copy link
Contributor Author

jlewi commented May 14, 2024

@sourishkrout PTAL

Copy link
Member

@sourishkrout sourishkrout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directionally this looks right to me 👍 .

However, I'm working to get #575 merged so we can have the API definitions all in one place and avoid duplication. Looking to merge the upstream PR before EoD.

If you could go ahead and rebase, and move the ai definitions into api please?

@sourishkrout
Copy link
Member

sourishkrout commented May 14, 2024

@jlewi main line is merged. you'll see that now everything API lives under pkg/api. the definitions specifically under pkg/api/proto and generated code under pkg/api/gen.

PS: the pre commit hooks are unhappy with the PR as is. however, once ai is moved under pkg/api/proto/ai it should resolve accordingly.

@sourishkrout
Copy link
Member

The PR currently defines a new buf module for the AI service. I'm not sure if that's required or advisable.

I'd suggest to just make it part of Runme's buf module for now. We can work on untangling once we have a working "prototype".

@jlewi
Copy link
Contributor Author

jlewi commented May 14, 2024

@sourishkrout Thanks for the quick refactor. I've updated the PR to move the ai protos into the pkg/api directory.

@jlewi
Copy link
Contributor Author

jlewi commented May 15, 2024

The SONAR failure seems unrelated to my changes. The error is
Set the SONAR_TOKEN env variable.

Copy link
Member

@sourishkrout sourishkrout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@sourishkrout
Copy link
Member

The SONAR failure seems unrelated to my changes. The error is Set the SONAR_TOKEN env variable.

Something with repo secrets permissions since your repos is a fork. Not to worried about this. Sonar is a way for us to monitor code health from a mile high view.

@sourishkrout sourishkrout merged commit 7fd1591 into stateful:main May 15, 2024
4 of 5 checks passed
jlewi added a commit to jlewi/foyle that referenced this pull request May 17, 2024
* Define a GRPC service that will take a RunMe notebook and generate
cells to be inserted into that notebook
  * The gRPC service is being defined in stateful/runme#573
* Define converters to and from RunMe protos to Foyle Protos
* Make a copy of RunMe's ULID generator code so we can generate cell ids
in the format RunMe expects
* RunMe"s ULID generator code is defined in an internal package so we
can't just reuse it.
* Related to [tn_004.md runme
integration](https://github.com/jlewi/foyle/blob/main/tech_notes/tn004_runme.md)
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