-
Notifications
You must be signed in to change notification settings - Fork 473
Fix DeprecationWarning when encoding StripeObject metadata (fixes #1651) #1687
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
base: master
Are you sure you want to change the base?
Fix DeprecationWarning when encoding StripeObject metadata (fixes #1651) #1687
Conversation
|
@praniketkw Can you also update the PR description based on the new approach? |
|
@ramya-stripe Done! |
| if value is None: | ||
| continue | ||
| elif hasattr(value, "stripe_id"): | ||
| yield (key, value.stripe_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @praniketkw
Just thought of an alternative with a smaller footprint. Can you try this and see if it works?
elif hasattr(value, "stripe_id")
yield (key, getattr(value, "id"))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion @ramya-stripe! I implemented your approach using getattr(value, "id") instead of value.stripe_id.
I had to wrap the hasattr(value, "stripe_id") check in warnings.catch_warnings() since hasattr itself triggers the deprecation warning. Also added a check for getattr(value, "id") is not None to handle edge cases properly.
The implementation now uses your approach while ensuring no warnings bubble up during internal encoding. Let me know if this looks good!
Fixes #1651
Why?
When updating metadata on resources like invoices, users encountered a DeprecationWarning even though they were using the public API correctly. The warning was triggered internally when the encoder checked for the deprecated stripe_id property on StripeObject instances. While the deprecation warning serves an important purpose for direct user access to stripe_id, it should not be shown for internal library usage.
What?