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.

Sign up for GitHub

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

Merged
merged 19 commits into from
Oct 26, 2024
Merged

Add include_deleted param to ledger_entry API #721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
29468e7
add nfts_by_issuer
May 16, 2024
208d969
update Changelog
Kassaking7 May 16, 2024
a2fe883
added nfts_by_issuer to Request's get_method
Kassaking7 May 22, 2024
20885b3
reformat changelog and add required comment for nfts_by_issuer.py
Kassaking7 May 22, 2024
d70f8fe
Merge branch 'main' into main
Kassaking7 Jun 11, 2024
1038ef7
add include_delete field
Kassaking7 Jun 14, 2024
9b3ae2e
update change log
Kassaking7 Jun 14, 2024
8dd215c
Merge pull request #1 from Kassaking7/add-include-delete-to-ledger-en…
Kassaking7 Jul 3, 2024
f7b8144
fix conflict
Kassaking7 Jul 3, 2024
910fb14
another conflict fixed
Kassaking7 Jul 3, 2024
e957057
Merge branch 'XRPLF:main' into main
Kassaking7 Jul 9, 2024
23dcc9b
add a comment
Kassaking7 Jul 10, 2024
b2b12c1
style fix
Kassaking7 Jul 10, 2024
4baff7c
Merge branch 'XRPLF:main' into main
Kassaking7 Jul 10, 2024
cda547b
fix a style issue
Kassaking7 Jul 10, 2024
945f018
changed to more descriptive comment
Kassaking7 Jul 10, 2024
ce4fe99
Merge branch 'main' into main
ckeshava Jul 22, 2024
1512949
Merge branch 'main' into main
mvadari Oct 25, 2024
28db48a
update CHANGELOG
Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [[Unreleased]]

### Added
- Add `include_deleted` to ledger_entry request

### BREAKING CHANGE:
- Remove Python 3.7 support to fix dependency installation and use 3.8 as new default.

Expand Down
2 changes: 2 additions & 0 deletions xrpl/models/requests/ledger_entry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Kassaking7 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

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

Copy link
Contributor Author

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Python doesn't have response models.

"""This parameter is supported only by Clio servers"""
Copy link
Collaborator

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?

Copy link
Collaborator

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/

Copy link
Contributor Author

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?

Copy link
Collaborator

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.


def _get_errors(self: Self) -> Dict[str, str]:
errors = super()._get_errors()
Expand Down
Loading