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

Generate explicit nested resource class methods #1050

Merged

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Sep 13, 2023

Summary

Replaces @nested_resource_class_methods decorator with explicit method definitions. This will make things easier to type.

To make this truly a no-op, we need to preserve the "helper" methods like stripe.Customer.balance_transactions_url and stripe.Customer.balance_transactions_request, even though we are no longer using them. Hence, we leave the @nested_resource_class_methods decorator in without any operations to provide these methods.

Testing

How can we be sure this doesn't change any behavior?

I have a little test script I added in Test nested resources that I ran both with and without the change.

git checkout richardm-explicit-nested-resource-class-methods-comparison
pip install -e .
python3 call_nested_resource_methods.py > /tmp/old
git checkout richardm-explicit-nested-resource-class-methods~1
pip install -e .
python3 call_nested_resource_methods.py > /tmp/new
sha256sum /tmp/old
# cb8e3ed1192dfe52df8d021e5a693ec55f0cca78d762f1a9bb75b87168a584ba  /tmp/old
sha256sum /tmp/new
# cb8e3ed1192dfe52df8d021e5a693ec55f0cca78d762f1a9bb75b87168a584ba  /tmp/new
wc -l /tmp/old
#     240 /tmp/old

(240 = 40 nested methods * 2 log statements * 3 test cases each)

This ~proves that, though there are a few differences in terms of whether the data is on the params dictionary or whether it is taken off via util.read_special_variable the behavior of everything is the same once it gets to api_requestor.

@richardm-stripe richardm-stripe marked this pull request as draft September 13, 2023 23:15
@richardm-stripe richardm-stripe force-pushed the richardm-explicit-nested-resource-class-methods branch from 8d8f998 to 19b4c69 Compare September 14, 2023 00:12
@richardm-stripe richardm-stripe changed the title [WIP] Generate explicit nested resource class methods Generate explicit nested resource class methods Sep 14, 2023
@richardm-stripe richardm-stripe marked this pull request as ready for review September 14, 2023 00:41
@richardm-stripe richardm-stripe enabled auto-merge (squash) September 14, 2023 18:11
@richardm-stripe richardm-stripe merged commit 57f8b88 into master Sep 14, 2023
10 checks passed
@remi-stripe remi-stripe deleted the richardm-explicit-nested-resource-class-methods branch September 28, 2023 23:11
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