Skip to content

add support for v2 deleted objects #1543

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

Open
wants to merge 4 commits into
base: sdk-release/next-major
Choose a base branch
from

Conversation

xavdid-stripe
Copy link
Member

Why?

The typing of delete calls has always been a little confusing in V1: SDKs say they return an entire object (like a Customer) but with the deleted property set to True. That's true for some APIs but not for others. For consistency, we just always say you get a "real" object.

V2 APIs are more strict about the data they return: there's only ever id and object. To better represent this, we're now generating a V2.DeletedObject class that is returned from all v2 delete calls. This PR includes that generated class, but APIRequestor-related code to use it when appropreate.

Note: this PR is generated using the V2 OpenAPI spec rather than the protos so that the DeletedObject class is generated.

What?

  • add param to _convert_to_stripe_object
  • provide that param from the API requestor
  • add tests

See Also

DEVSDK-2588

@xavdid-stripe xavdid-stripe marked this pull request as ready for review August 6, 2025 23:03
@xavdid-stripe xavdid-stripe requested a review from a team as a code owner August 6, 2025 23:03
@xavdid-stripe xavdid-stripe requested review from helenye-stripe and removed request for a team August 6, 2025 23:03
@xavdid-stripe xavdid-stripe changed the base branch from sdk-release/next-major to sdk-releases/next-major August 6, 2025 23:21
@helenye-stripe
Copy link
Contributor

I'm peaceful with this but won't the generated changes get removed by CI since we'll go back to using the proto spec to generate the SDK?

@xavdid-stripe xavdid-stripe changed the base branch from sdk-releases/next-major to sdk-release/next-major August 7, 2025 21:03
@xavdid-stripe
Copy link
Member Author

I'm peaceful with this but won't the generated changes get removed by CI since we'll go back to using the proto spec to generate the SDK?

the manual changes won't. Plus I don't think we're running codegen against sdk-release/next-major. Worst case scenario, CI fails beacuse it's using this code against the proto generation, but I think it won't (because of the branch). The ordering is definitely tricky there though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants