feat/enterpriseportal: ArchiveEnterpriseSubscription RPC definition#63330
Conversation
582f64c to
a6f74da
Compare
| // ArchiveEnterpriseSubscriptionRequest archives an existing Enterprise | ||
| // subscription. This is a permanent operation, and cannot be undone. | ||
| rpc ArchiveEnterpriseSubscription(ArchiveEnterpriseSubscriptionRequest) returns (ArchiveEnterpriseSubscriptionResponse) { |
There was a problem hiding this comment.
@eseliger - I suppose archiving a subscription, also automatically revokes all licenses corresponding to a subscription? (presumably revocation is used for the "online license checks" that Sourcegraph instances run)
There was a problem hiding this comment.
There was a problem hiding this comment.
It isn't "revoking" them, but they are disabled and should not be valid anymore.
cc @pjlast
There was a problem hiding this comment.
🤔 what is the difference between "revoked" and "disabled" in the context of license keys if they are not valid anymore?
There was a problem hiding this comment.
Hm, yeah it would help to understand the desired behaviour to licenses when a subscription is archived. Should I align on the naming of the verb? ("revoke" for both? that feels a bit strange though)
There was a problem hiding this comment.
I don't think it makes sense for archival to not revoke all licenses immediately IMO, added docstring describing revocation of all licenses as well
| // ArchiveEnterpriseSubscriptionRequest archives an existing Enterprise | ||
| // subscription. This is a permanent operation, and cannot be undone. | ||
| rpc ArchiveEnterpriseSubscription(ArchiveEnterpriseSubscriptionRequest) returns (ArchiveEnterpriseSubscriptionResponse) { |
There was a problem hiding this comment.
🤔 what is the difference between "revoked" and "disabled" in the context of license keys if they are not valid anymore?
…tion (#63331) Enterprise Portal equivalent of the `revokeLicense` GraphQL mutation. It's not a "delete" operation - we currently retain the record forever - so it's idempotent, as the revocation of an already-revoked license can return OK. If we want to add a hard delete we should add a separate one based more closely on https://google.aip.dev/135. Related: https://github.com/sourcegraph/sourcegraph/pull/63330 Closes CORE-153 ## Test plan CI
…ore-153-enterprise-portal-proto-rpc-for-archiveproductsubscription
There's a confusing notion of "archived license" that really means "archived subscription", which is problematic because "can an archived subscription, have valid licenses, in a world where revoked licenses exist?" IMO archiving a subscription should immediately and permanently revoke all its associated licenses, per discussion in https://github.com/sourcegraph/sourcegraph/pull/63330#discussion_r1645333457. This means we can remove all notion of "archived license" - when looking at licenses, they're only revoked, or not revoked.⚠️ These RPCs are not used anywhere yet so this is a safe breaking change. ## Test plan CI
Enterprise Portal equivalent of the
archiveProductSubscriptionGraphQL mutation. It's not a "delete" operation - we retain the record forever - so it's idempotent, as an archive on an already-archived subscription can return OK. If we want to add a hard delete we should add a separate one based more closely on https://google.aip.dev/135.It's not part of "update" because it's closer to a "delete" than anything else, and currently cannot be undone.
Closes CORE-153
Test plan
CI