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

Track usage of deprecated save #1146

Merged
merged 1 commit into from
Dec 13, 2023
Merged

Track usage of deprecated save #1146

merged 1 commit into from
Dec 13, 2023

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Nov 21, 2023

Note: not to be merged until after #1142 so as to minimize merge conflicts.

Changelog

  • Reports uses of the deprecated .save in X-Stripe-Client-Telemetry. (You can disable telemetry via stripe.enable_telemetry = false, see the README.)

Details

The unsatisfying thing about the implementation is how the telemetry header is added at the http_client layer of abstraction. I would have liked to change this and moved it to api_requestor -- but we are planning to fundamentally upheave api_requestor in the near future anyway, so I chose not to.

I added _usage as an optional keyword param to ApiRequestor.request and HttpClient.request_with_retries (along with streaming analogs, and to internal methods).

Testing

I modified the existing test case for telemetry (in integration_test.py) to use a method that would trigger "save" usage to be included.

@richardm-stripe richardm-stripe marked this pull request as draft November 21, 2023 00:48
@richardm-stripe richardm-stripe force-pushed the richardm-usage branch 3 times, most recently from b3d9149 to 55aac4d Compare December 1, 2023 22:11
@richardm-stripe richardm-stripe marked this pull request as ready for review December 1, 2023 22:36
stripe/http_client.py Outdated Show resolved Hide resolved
Copy link
Contributor

@anniel-stripe anniel-stripe left a comment

Choose a reason for hiding this comment

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

Left a couple comments but nothing blocking 👍

@richardm-stripe richardm-stripe merged commit 4464b6a into master Dec 13, 2023
15 checks passed
@richardm-stripe richardm-stripe deleted the richardm-usage branch January 26, 2024 16:57
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.

2 participants