-
Notifications
You must be signed in to change notification settings - Fork 438
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
richardm-stripe
commented
Oct 26, 2023
richardm-stripe
force-pushed
the
richardm-mark-internal-methods
branch
from
October 26, 2023 13:36
5283262
to
22eb8ce
Compare
richardm-stripe
force-pushed
the
richardm-mark-internal-methods
branch
from
November 20, 2023 20:19
22eb8ce
to
aec3d69
Compare
richardm-stripe
force-pushed
the
richardm-mark-internal-methods
branch
2 times, most recently
from
December 11, 2023 19:29
bc9ea78
to
364f6db
Compare
richardm-stripe
force-pushed
the
richardm-mark-internal-methods
branch
from
December 11, 2023 19:34
364f6db
to
2127f66
Compare
richardm-stripe
commented
Dec 11, 2023
richardm-stripe
commented
Dec 11, 2023
richardm-stripe
commented
Dec 11, 2023
richardm-stripe
commented
Dec 11, 2023
richardm-stripe
requested review from
a team and
pakrym-stripe
and removed request for
a team
December 11, 2023 22:00
pakrym-stripe
approved these changes
Dec 11, 2023
richardm-stripe
commented
Dec 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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 inStripeObject.__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 theurl
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 2018Internal methods
APIRequestor.format_app_info
- internal helper used for formatting the user agent string.ListObject.list
,SearchResultObject.search
- LikeListObject.retrieve
andListObject.create
-- makes a request based off of theurl
property. This one is used internally by pagination, but for external users it's better if they either auto-paginate or call the specificlist
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.strStripeObject.stripe_id
- internal helper for APIRequestor._api_encodeNot 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 forCustomer.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.