Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

feat/enterpriseportal: UpdateCodyGatewayAccess RPC definition#63307

Merged
bobheadxi merged 4 commits into
mainfrom
robert/core-154-enterprise-portal-proto-rpc-for-updating-cody-gateway-access
Jun 18, 2024
Merged

feat/enterpriseportal: UpdateCodyGatewayAccess RPC definition#63307
bobheadxi merged 4 commits into
mainfrom
robert/core-154-enterprise-portal-proto-rpc-for-updating-cody-gateway-access

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Jun 18, 2024

Copy link
Copy Markdown
Member

Proposed RPC definition for updating Cody Gateway access, which is currently part of the updateProductSubscription GraphQL mutation, via Enterprise Portal. The RPC follows patterns suggested for "update" RPCs in https://google.aip.dev/134.

As an example, to use UpdateCodyGatewayAccess to update a subscription's code completions limit:

UpdateCodyGatewayAccess({
  access: {
    subscription_id: "es_...",
    code_completions: { limit: 420 },
  },
  update_mask: {
    paths: ["code_completions.limit"],
  },
})  

Closes CORE-154. Implementation is tracked in a follow-up (CORE-159) - for now this will just return "unimplemented".

I've also removed the "draft state" docstrings.

Test plan

CI

@bobheadxi bobheadxi requested review from a team June 18, 2024 03:39
@cla-bot cla-bot Bot added the cla-signed label Jun 18, 2024
// UpdateEnterpriseSubscription updates the Cody Gateway access granted to an
// Enterprise subscription.
//
// Only properties specified by the update_mask are applied.

@unknwon unknwon Jun 18, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After last time we discussed in Slack based on the following

image

My understanding of "AIP" principle for update is:

  1. When the update_mask is absent, non-empty fields are updated, e.g.
    CleanShot 2024-06-18 at 09 36 01@2x
  2. When specified, fields are updated by the list of "path"s, e.g.
    CleanShot 2024-06-18 at 09 36 45@2x
  3. When * is used as "path", force update all fields, e.g.
    CleanShot 2024-06-18 at 09 37 12@2x

Following that, in the RPC docstring, we'd better to say which exact paths are updatable, e.g. (you're doing it already in the PR)

CleanShot 2024-06-18 at 09 38 03@2x

All of which is to say "Only properties specified by the update_mask are applied." should be removed 😂

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Makes sense! I copied this from the existing UpdateEnterpriseSubscription 😛 I'll update both

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The one for UpdateEnterpriseSubscription is taken cared in my PR, maybe just update this one so either PR merges would not create conflict 😁

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ah, okay haha

@bobheadxi bobheadxi merged commit d0ea292 into main Jun 18, 2024
@bobheadxi bobheadxi deleted the robert/core-154-enterprise-portal-proto-rpc-for-updating-cody-gateway-access branch June 18, 2024 19:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants