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

DiscoveryEngineDataStore: mockgcp support #3003

Conversation

justinsb
Copy link
Collaborator

@justinsb justinsb commented Oct 24, 2024

  • DiscoveryEngineDataStore: mockgcp support
  • mockgcp: generate code for discoveryengine
  • DiscoveryEngineDataStore: protos for mockgcp
  • DiscoveryEngineDataStore: golden output for mockgcp

@yuwenma
Copy link
Collaborator

yuwenma commented Oct 25, 2024

/approve

The mock gcp part (last 3 commits) looks good. Waiting for the diff base PR to merge

@justinsb justinsb force-pushed the discoveryenginedatastore_mockgcp branch from 6b3b111 to fa734c8 Compare October 26, 2024 14:16
@yuwenma
Copy link
Collaborator

yuwenma commented Oct 26, 2024

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Oct 26, 2024
@justinsb justinsb force-pushed the discoveryenginedatastore_mockgcp branch from fa734c8 to 3d38bf6 Compare October 31, 2024 20:52
@google-oss-prow google-oss-prow bot removed the lgtm label Oct 31, 2024
@@ -56,8 +56,7 @@ X-Xss-Protection: 0
"defaultSchemaId": "default_schema",
"displayName": "My first data store",
"industryVertical": 1,
"name": "projects/${projectNumber}/locations/global/collections/default_collection/dataStores/datastore-${uniqueId}",
"servingConfigDataStore": {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want this servingConfigDataStore field showing in the GCP object.

Copy link
Collaborator Author

@justinsb justinsb Nov 1, 2024

Choose a reason for hiding this comment

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

It's not in the protos yet (at least not that I could find)

Copy link
Collaborator

@yuwenma yuwenma left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

One nit, not a blocker

@google-oss-prow google-oss-prow bot added the lgtm label Oct 31, 2024
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yuwenma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@justinsb justinsb force-pushed the discoveryenginedatastore_mockgcp branch from 3d38bf6 to 5c6e1d7 Compare November 1, 2024 13:02
@google-oss-prow google-oss-prow bot removed the lgtm label Nov 1, 2024
@yuwenma
Copy link
Collaborator

yuwenma commented Nov 2, 2024

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Nov 2, 2024
@google-oss-prow google-oss-prow bot merged commit f17f547 into GoogleCloudPlatform:master Nov 2, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants