-
Notifications
You must be signed in to change notification settings - Fork 84
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
Add include_deleted
param to ledger_entry API
#721
base: main
Are you sure you want to change the base?
Conversation
…try-api Add include delete to ledger entry api
@@ -264,6 +264,8 @@ class LedgerEntry(Request, LookupByLedgerRequest): | |||
binary: bool = False | |||
nft_page: Optional[str] = None | |||
"""Must be the object ID of the NFToken page, as hexadecimal""" | |||
include_deleted: Optional[bool] = None | |||
"""This parameter is supported only by Clio servers""" |
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.
can you also add a link to the documentation reference for this include_deleted
parameter?
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.
I couldn't find a mention of include_deleted
in the XRPL docs: https://xrpl.org/docs/references/http-websocket-apis/public-api-methods/ledger-methods/ledger_entry/
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.
This one is still on progress (XRPLF/clio#1306) so it's not in XRPL docs for now. Do I need to add that link to ledger_entry.py?
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.
Hmm, I see. It would have been helpful to add the docs link, but it's not absolutely essential.
@@ -264,6 +264,8 @@ class LedgerEntry(Request, LookupByLedgerRequest): | |||
binary: bool = False | |||
nft_page: Optional[str] = None | |||
"""Must be the object ID of the NFToken page, as hexadecimal""" | |||
include_deleted: Optional[bool] = None |
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.
Why does the xrpl-py client library not contain the deleted_ledger_index
parameter? I observed it in the javascript client library
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.
Since there's no response class in ledger_entry.py so I assume we don't need it? For js one there's already a response class(interface) so I just add deleted_ledger_index
to that class.
High Level Overview of Change
Based on XRPLF/clio#1306
Context of Change
Type of Change
Did you update CHANGELOG.md?
Test Plan