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

gRPC API surface plus response/requests datatype naming conventions #5

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

jasonmeverett
Copy link
Collaborator

gRPC API Surface

This PR introduces a gRPC servicer class that can be used to implement the application logic API. This is currently implemented in ft/app.py, where the API will remain until we finalize implementing the gRPC layer (in a future PR).

import grpc
from ft.proto import fine_tuning_studio_pb2_grpc
from ft.service import FineTuningStudioApp
from multiprocessing import Process
Copy link
Collaborator

Choose a reason for hiding this comment

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

What multiprocessing are we planning to implement?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was an old artifact from trying different ways of spawning the gRPC server. Originally I was trying to spawn the gRPC server as a sub-process to the Streamlit server, but this was causing some headache because streamlit reruns entry point scripts upon every interaction.

We can probably safely remove this in the future.

Copy link
Collaborator

@Abhishek-Rnjn Abhishek-Rnjn left a comment

Choose a reason for hiding this comment

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

LGTM

@jasonmeverett jasonmeverett merged commit 83eb7c8 into dev Aug 7, 2024
@jasonmeverett jasonmeverett deleted the grpc branch August 7, 2024 17:51
jasonmeverett pushed a commit that referenced this pull request Nov 1, 2024
Added dataset bug fix. Where data will be taken of 1st adapter only
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