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

chore: Check in service proto file #1174

Merged
merged 2 commits into from
May 11, 2022
Merged

Conversation

yan283
Copy link
Contributor

@yan283 yan283 commented Apr 20, 2022

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
@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

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
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
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check in match service definition
4 participants