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

implement gRPC rest gateway #39

Closed
wants to merge 1 commit into from
Closed

implement gRPC rest gateway #39

wants to merge 1 commit into from

Conversation

lampajr
Copy link
Contributor

@lampajr lampajr commented Sep 27, 2023

Description

Fixes opendatahub-io/model-registry#32

Automate the generation of a gRPC rest gateway proxy in front of the actual gRPC server.

Startup the gateway:

$ ./model-registry gateway

or

$ make run/gateway

NOTE: right now I setup a 10s timeout on gRPC gateway, this means that if you start it up and the gRPC is not running yet it will fail and exit after 10s

How Has This Been Tested?

In order to test this I created a new python script test/python/test_mlmetadata_rest.py that aims to create some ArtifactType and an Artifact instance dealing with the REST interface instead of the gRPC one.

Steps to run the tests:

# in one terminal
$ make run/server

# in another terminal
$ make run/gateway

# in third terminal
$ python test/python/test_mlmetadata_rest.py

Merge criteria:

  • The commits and have meaningful messages; the author will squash them after approval or will ask to merge with squash.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@rareddy
Copy link
Contributor

rareddy commented Sep 28, 2023

I there a reason why we want this to be a different process from the main gRPC server? Can not wire up such that it is a single process?

@dhirajsb
Copy link
Contributor

@lampajr this looks quite good 👍
I'll review it in more detail tomorrow.

@lampajr
Copy link
Contributor Author

lampajr commented Sep 29, 2023

I there a reason why we want this to be a different process from the main gRPC server? Can not wire up such that it is a single process?

Initially because as per my understanding the gRPC gateway needs to dial a connection to the gRPC server (that is why I thought having different processes would have been a better approach) - on the other hand I am evaluating if we can sort this out having a single process as well.

Copy link
Contributor

@dhirajsb dhirajsb left a comment

Choose a reason for hiding this comment

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

@lampajr let's hold on to this PR for now while we investigate the envoy proxy that google's ml-metadata server uses.
@tarilabs noticed that it might be exposing an openapi v3 service, which would be preferable.

@lampajr lampajr marked this pull request as draft October 4, 2023 07:34
@lampajr
Copy link
Contributor Author

lampajr commented Oct 12, 2023

Closing as not needed for now.

@lampajr lampajr closed this Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

[feature][grpc] Experiment gRPC gateway
3 participants