-
Notifications
You must be signed in to change notification settings - Fork 180
fix: make start_ledger
optional and mutually exclusive with cursor
#1032
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
Conversation
Hi @aolieman, please remember to update the |
I've updated the changelog. This might be complete. What do you think? |
Hi @aolieman, it's late here, so I need to wait until tomorrow to review it. |
I think you need to run |
Alright, I understand. I'll have a look at the CI output in the meantime. Depending on which type checker I use, I still get warnings for optional arguments that don't have the None type: # pyright warns about all these default values
start_ledger: int = None,
filters: Sequence[EventFilter] = None,
cursor: str = None,
limit: int = None, Is this intentional? I can change it if you want, but it's not new and I don't think it's a big issue for anyone. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1032 +/- ##
=======================================
Coverage 98.27% 98.28%
=======================================
Files 262 262
Lines 14250 14308 +58
=======================================
+ Hits 14004 14062 +58
Misses 246 246
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
I don't think this is a problem; let's keep it as it is in this PR. |
👍 The other parts look good; we just need to fix the issue that it cannot run on py 3.8 to py 3.10 before we can merge it. |
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.
🎉 Thank you for your contribution.
It was a pleasure working with you! |
Fix for #1031
The
start_ledger
argument needs to be optional on RPC methods that support pagination. I've added model validation that enforces the mutual exclusivity with thecursor
argument. This helps the SDK user see the error faster than if they'd get it from the RPC response.