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

Mark defunct and internal methods as deprecated #1101

Merged
merged 13 commits into from
Dec 12, 2023

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Oct 26, 2023

Summary

This PR adds deprecation annotations to classes and methods that, in my judgement, we should phase out -- at least for external users.

For things we want to keep internally but deprecate externally, I took two different approaches in different situations.

  1. In some cases, I added a deprecation notice, introduced an internal (underscore-prefixed) version, and moved all internal usages to the underscore-prefixed copy.
  2. In other cases, I just added the deprecation notice and added a with warnings.catch_warnings(): clause around the internal use, so that errors won't bubble down to users.

Option 2 is more conservative. Both options preserve the behavior of the function itself. However, with Option 1 could cause a behavior difference if somebody had a child class, overrode the method in question, and relied on their overloaded version to be called by the internal code in the parent class. I've used option 2 in circumstances where that was a concern.

Internal Classes

  • StripeObject.ReprJSONEncoder - Used in StripeObject.__str__ for custom encoding datetimes. No external use case.

Deprecated methods

  • APIResource.retrieve, - Superceded by explicit overrides on the child class.
  • ListObject.retrieve, ListObject.create - odd methods that use the url from the ListObject to construct post and get requests. There is no guarantee that the API will actually support those methods, and I think it's better for users to be explicit about which API method they are calling.
  • StripeObject.api_base - I don't think this has been used since 2018

Internal methods

  • APIRequestor.format_app_info - internal helper used for formatting the user agent string.
  • ListObject.list, SearchResultObject.search - Like ListObject.retrieve and ListObject.create -- makes a request based off of the url property. This one is used internally by pagination, but for external users it's better if they either auto-paginate or call the specific list method explicitly.
  • ListObject.empty_list, SearchResultObject.empty_search_result - internal helper for pagination, no real legitimate external use.
  • StripeError.construct_error_object - internal helper for deserializing errors.
  • StripeObject.to_dict_recursive - internal helper for StripeObject.str
  • StripeObject.stripe_id - internal helper for APIRequestor._api_encode

Not deprecated (yet)

  • ListableAPIResource, CreateableAPIResource, DeleteableAPIResource, SearchableAPIResource, SingletonAPIResource, UpdateableAPIResource, @custom_method, @nested_resource_class_methods - These classes provide CRUDL methods that have all been superceded by overrides on the resources that inherit from them, but they are still in use by other parts of the library, so it's difficult to prevent DeprecationWarning from being bubbled up when users use those other, non-deprecated parts of the library that use these features.
  • ListableResource.auto_paging_iter - I almost deprecated this one. Customer.auto_paging_iter(...) is just shorthand for Customer.list(...).auto_paging_iter() and feels unnecessary and non-standard, but I don't think preserving it is too harmful and it's not worth forcing users to change.

stripe/util.py Outdated Show resolved Hide resolved
@richardm-stripe richardm-stripe force-pushed the richardm-mark-internal-methods branch 2 times, most recently from bc9ea78 to 364f6db Compare December 11, 2023 19:29
@richardm-stripe richardm-stripe requested review from a team and pakrym-stripe and removed request for a team December 11, 2023 22:00
stripe/_error.py Outdated Show resolved Hide resolved
@richardm-stripe richardm-stripe enabled auto-merge (squash) December 12, 2023 00:11
@richardm-stripe richardm-stripe merged commit bf78301 into master Dec 12, 2023
14 checks passed
@richardm-stripe richardm-stripe deleted the richardm-mark-internal-methods 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.

None yet

2 participants