Skip to content

chore: Check in service proto file #1174

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 11, 2022
Merged

chore: Check in service proto file #1174

merged 2 commits into from
May 11, 2022

Conversation

yan283
Copy link
Contributor

@yan283 yan283 commented Apr 20, 2022 •
edited
Loading

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #1173 🦕
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Apr 20, 2022
@morgandu morgandu requested review from ivanmkc and sasha-gitg April 20, 2022 18:25
@sasha-gitg sasha-gitg added do not merge Indicates a pull request not ready for merge, due to either quality or timing. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Apr 20, 2022
@ivanmkc
Copy link
Contributor

ivanmkc commented Apr 27, 2022

The proto looks good but what was the rationale to not just add a match method for IndexEndpointServiceClient?
@yan283
Copy link
Contributor Author

yan283 commented Apr 27, 2022

Match is a method of MatchService, not IndexEndpointService. I assume that means it should not be part of IndexEndpointServiceClient?
@ivanmkc
Copy link
Contributor

ivanmkc commented Apr 28, 2022

Sorry, I mean MatchService. Will there be a GAPIC version of MatchService?
@yan283
Copy link
Contributor Author

yan283 commented Apr 28, 2022 •
edited
Loading

This is a short term solution to make it easy for clients to generate their own library. We are looking into options to publish GAPICs in the long run.
@ivanmkc
Copy link
Contributor

ivanmkc commented May 4, 2022

As discussed, please move to the _proto folder after #1192 is merged.
@product-auto-label product-auto-label bot added the api: vertex-ai Issues related to the googleapis/python-aiplatform API. label May 5, 2022
@sararob sararob added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 5, 2022
@ivanmkc
Copy link
Contributor

ivanmkc commented May 9, 2022

Hi @yan283, the matching engine PR is merged so you can move the proto to the google/cloud/aiplatform/matching_engine/_protos folder.
@sararob sararob removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 10, 2022
ivanmkc
ivanmkc approved these changes May 10, 2022
View reviewed changes
Copy link
Contributor

@ivanmkc ivanmkc left a comment

Choose a reason for hiding this comment

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

LGTM
@yan283 yan283 force-pushed the match-service branch 2 times, most recently from 4d48aa4 to 82f0584 Compare May 10, 2022 20:55
@ivanmkc ivanmkc changed the title Check in service proto file chore: Check in service proto file May 11, 2022
@ivanmkc ivanmkc merged commit 5fdf151 into googleapis:main May 11, 2022
@release-please release-please bot mentioned this pull request May 11, 2022
Merged
@release-please release-please bot mentioned this pull request Jun 8, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: vertex-ai Issues related to the googleapis/python-aiplatform API. size: m Pull request size is medium.

4 participants

@yan283 @ivanmkc @sararob @sasha-gitg