✨ add `order_by` and `descending` options to scan and fetch_all queries #291

ff137 · 2024-07-25T10:21:44Z

Closes #290

Adds ordering functionality to scan and fetch_all queries, to allow for custom ordering of fetched storage records.

Signed-off-by: ff137 <[email protected]>

ff137 · 2024-07-26T12:16:42Z

Ready for review!

I wanted to test compatibility with ACA-Py, but building and installing the python wrapper from my local branch, and running pytest in ACA-Py, gives me:

...
INTERNALERROR>     get_library()
INTERNALERROR>   File "/root/anaconda3/envs/acapy/lib/python3.12/site-packages/aries_askar/bindings/__init__.py", line 44, in get_library
INTERNALERROR>     LIB.loaded
INTERNALERROR>   File "/root/anaconda3/envs/acapy/lib/python3.12/site-packages/aries_askar/bindings/lib.py", line 352, in loaded
INTERNALERROR>     self._lib = LibLoad(self.__class__.LIB_NAME)
INTERNALERROR>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
INTERNALERROR>   File "/root/anaconda3/envs/acapy/lib/python3.12/site-packages/aries_askar/bindings/lib.py", line 106, in __init__
INTERNALERROR>     self._load_library()
INTERNALERROR>   File "/root/anaconda3/envs/acapy/lib/python3.12/site-packages/aries_askar/bindings/lib.py", line 129, in _load_library
INTERNALERROR>     raise AskarError(
INTERNALERROR> aries_askar.error.AskarError: Library not found in path: aries_askar

This resolves when I revert to installing official pypi package with pip install aries_askar. So, there must be some extra step required when calling pip install . in the aries-askar/wrappers/python dir.

I tried adding aries-askar/target/release to LD_LIBRARY_PATH, but no dice.

If anyone can assist or direct me on how to set this up for local testing with ACA-Py, will be much appreciated!

ff137 · 2024-07-26T12:21:34Z

A logical extension would be to add the custom ordering option to "fetch all" queries as well (in a future PR)

ff137 · 2024-08-05T18:20:09Z

Good news! Managed to successfully test the changes locally 🚀
after copying target/release/libaries_askar.so to wrappers/python/aries_askar 🚚

Happy to confirm that all the ACA-Py tests pass using this branch's changes ✅
Will begin implementing the order_by functionality for ACA-Py soon™️

askar-storage/src/backend/db_utils.rs

Signed-off-by: ff137 <[email protected]>

ff137 · 2024-08-15T17:17:21Z

I think I'm gonna try add the ordering options for fetching all records -- not just for scanning.
Makes sense imo to include it all in one PR

Signed-off-by: ff137 <[email protected]>

ff137 · 2024-08-16T20:09:59Z

I've expanded ordering options to the fetch_all method as well.

I can confirm python wrapper works with changes (ACA-Py tests pass using this build).

JavaScript wrapper still has some tests that remain to be fixed ...

Signed-off-by: ff137 <[email protected]>

ff137 · 2024-08-19T21:33:57Z

@andrewwhitehead Please let me know if you've had moment to review, if there are any additional steps necessary here. Maybe changing default behavior, or implementing some specific tests.

ACA-Py tests are passing with these changes here: openwallet-foundation/acapy#3173

And our end-to-end tests, asserting unique results across pages, are passing now (as described here:
openwallet-foundation/acapy#3173 (comment))

andrewwhitehead

Thanks, I think it looks good!

Signed-off-by: ff137 <[email protected]>

* ✨ add `order_by` and `descending` options to query / scan and fetch_all methods Signed-off-by: ff137 <[email protected]> * ✨ add `order_by` and `descending` options to PaginatedQuerySchema Signed-off-by: ff137 <[email protected]> * ✨ modify `get_limit_offset` to `get_paginated_query_params` Signed-off-by: ff137 <[email protected]> * ✨ add ordering to InMemoryStorage scan and fetch_all methods Signed-off-by: ff137 <[email protected]> * 🚧 test in-progress aries-askar PR: openwallet-foundation/askar#291 Signed-off-by: ff137 <[email protected]> * ⬆️ Update lock file Signed-off-by: ff137 <[email protected]> * 🎨 fix ruff warning Signed-off-by: ff137 <[email protected]> * ✅ fix assertions Signed-off-by: ff137 <[email protected]> * 🚧 test aries-askar with TestPyPI package Signed-off-by: ff137 <[email protected]> * 🚧 test latest askar testpypi package Signed-off-by: ff137 <[email protected]> * 🎨 Update order_by description and default value. Include in schema Signed-off-by: ff137 <[email protected]> * ⬆️ Update aries-askar test pypi package to pre-orjson feat release Signed-off-by: ff137 <[email protected]> --------- Signed-off-by: ff137 <[email protected]>

Signed-off-by: ff137 <[email protected]>

* ✨ add `order_by` and `descending` options to query / scan and fetch_all methods Signed-off-by: ff137 <[email protected]> * ✨ add `order_by` and `descending` options to PaginatedQuerySchema Signed-off-by: ff137 <[email protected]> * ✨ modify `get_limit_offset` to `get_paginated_query_params` Signed-off-by: ff137 <[email protected]> * ✨ add ordering to InMemoryStorage scan and fetch_all methods Signed-off-by: ff137 <[email protected]> * 🚧 test in-progress aries-askar PR: openwallet-foundation/askar#291 Signed-off-by: ff137 <[email protected]> * ⬆️ Update lock file Signed-off-by: ff137 <[email protected]> * 🎨 fix ruff warning Signed-off-by: ff137 <[email protected]> * ✅ fix assertions Signed-off-by: ff137 <[email protected]> * 🚧 test aries-askar with TestPyPI package Signed-off-by: ff137 <[email protected]> * 🚧 test latest askar testpypi package Signed-off-by: ff137 <[email protected]> * 🎨 Update order_by description and default value. Include in schema Signed-off-by: ff137 <[email protected]> * 🐛 fix deserialisation of descending: bool parameter Signed-off-by: ff137 <[email protected]> * ⏪ revert to testing 0.3.3b0 askar test package Signed-off-by: ff137 <[email protected]> * 🔥 remove unnecessary record sorts (now that askar sorts by id == sorting by created_at) Signed-off-by: ff137 <[email protected]> * ✅ fix test assertions -- wallet_list and connections_list no longer does additional sorting Signed-off-by: ff137 <[email protected]> * ⬆️ Update lock file Signed-off-by: ff137 <[email protected]> --------- Signed-off-by: ff137 <[email protected]>

ff137 added 6 commits July 25, 2024 13:20

🚧 add order_by and descending argument to scan

744cfe9

Signed-off-by: ff137 <[email protected]>

🎨 format

29018ee

Signed-off-by: ff137 <[email protected]>

🎨 use Ffi types for order_by and descending

f03ac9f

Signed-off-by: ff137 <[email protected]>

🎨 replace Option[bool] with bool

93bcae6

Signed-off-by: ff137 <[email protected]>

🎨 fix order_by and descending reference

1dadd5a

Signed-off-by: ff137 <[email protected]>

✨ add order_by and descending options to python Scan object

a38d6dc

Signed-off-by: ff137 <[email protected]>

ff137 force-pushed the feat/order-by-scan branch from f8bddd4 to d338cc0 Compare July 26, 2024 11:37

✨ add order_by and descending options to scan in javascript wrapper

cf5787e

Signed-off-by: ff137 <[email protected]>

ff137 force-pushed the feat/order-by-scan branch from d338cc0 to cf5787e Compare July 26, 2024 12:01

ff137 changed the title ~~🚧 WIP: add ordering options to scan query~~ ✨ add ordering options to scan query Jul 26, 2024

ff137 marked this pull request as ready for review July 26, 2024 12:10

andrewwhitehead reviewed Aug 7, 2024

View reviewed changes

askar-storage/src/backend/db_utils.rs Outdated Show resolved Hide resolved

ff137 marked this pull request as draft August 7, 2024 07:38

ff137 added 3 commits August 15, 2024 15:01

✨ implement OrderBy as Enum, with Default impl

aee343e

Signed-off-by: ff137 <[email protected]>

🎨 order_by_query now takes OrderBy enum directly, instead of Option.

974e996

Signed-off-by: ff137 <[email protected]>

✨ Only add ordering, and limit/offset, if the query starts with SELECT

1a37edf

Signed-off-by: ff137 <[email protected]>

ff137 marked this pull request as ready for review August 15, 2024 12:10

ff137 changed the title ~~✨ add ordering options to scan query~~ ✨ add order_by and descending options to scan and fetch_all queries Aug 16, 2024

✨ expand ordering functionality to fetch_all query

2efcade

Signed-off-by: ff137 <[email protected]>

ff137 requested a review from andrewwhitehead August 16, 2024 20:09

ff137 added 2 commits August 16, 2024 23:14

✅ fix tests

0a9dd4f

Signed-off-by: ff137 <[email protected]>

🐛 fix arg type order

6272b1f

Signed-off-by: ff137 <[email protected]>

ff137 added a commit to ff137/acapy that referenced this pull request Aug 16, 2024

🚧 test in-progress aries-askar PR: openwallet-foundation/askar#291

0e7940e

Signed-off-by: ff137 <[email protected]>

ff137 mentioned this pull request Aug 16, 2024

✨ Add ordering options to askar scan and fetch_all methods openwallet-foundation/acapy#3173

Draft

🐛 add missing arguments

bd11dfe

Signed-off-by: ff137 <[email protected]>

ff137 force-pushed the feat/order-by-scan branch from fe5c8a9 to bd11dfe Compare August 16, 2024 21:20

🎨 fix lint errors

7eb7763

Signed-off-by: ff137 <[email protected]>

ff137 added a commit to ff137/acapy that referenced this pull request Aug 19, 2024

🚧 test in-progress aries-askar PR: openwallet-foundation/askar#291

07a5c5a

Signed-off-by: ff137 <[email protected]>

andrewwhitehead approved these changes Aug 20, 2024

View reviewed changes

andrewwhitehead merged commit 75226af into openwallet-foundation:main Aug 20, 2024
29 checks passed

ff137 deleted the feat/order-by-scan branch August 20, 2024 22:33

ff137 added a commit to ff137/acapy that referenced this pull request Aug 21, 2024

🚧 test in-progress aries-askar PR: openwallet-foundation/askar#291

505dcd2

Signed-off-by: ff137 <[email protected]>

ff137 added a commit to ff137/acapy that referenced this pull request Aug 29, 2024

🚧 test in-progress aries-askar PR: openwallet-foundation/askar#291

ee024d2

Signed-off-by: ff137 <[email protected]>

TimoGlastra mentioned this pull request Sep 13, 2024

New release #309

Open

ff137 added a commit to ff137/acapy that referenced this pull request Oct 22, 2024

🚧 test in-progress aries-askar PR: openwallet-foundation/askar#291

d00e0dd

Signed-off-by: ff137 <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ add `order_by` and `descending` options to scan and fetch_all queries #291

✨ add `order_by` and `descending` options to scan and fetch_all queries #291

ff137 commented Jul 25, 2024 •

edited

Loading

ff137 commented Jul 26, 2024 •

edited

Loading

ff137 commented Jul 26, 2024

ff137 commented Aug 5, 2024 •

edited

Loading

ff137 commented Aug 15, 2024

ff137 commented Aug 16, 2024 •

edited

Loading

ff137 commented Aug 19, 2024

andrewwhitehead left a comment

✨ add order_by and descending options to scan and fetch_all queries #291

✨ add order_by and descending options to scan and fetch_all queries #291

Conversation

ff137 commented Jul 25, 2024 • edited Loading

ff137 commented Jul 26, 2024 • edited Loading

ff137 commented Jul 26, 2024

ff137 commented Aug 5, 2024 • edited Loading

ff137 commented Aug 15, 2024

ff137 commented Aug 16, 2024 • edited Loading

ff137 commented Aug 19, 2024

andrewwhitehead left a comment

Choose a reason for hiding this comment

✨ add `order_by` and `descending` options to scan and fetch_all queries #291

✨ add `order_by` and `descending` options to scan and fetch_all queries #291

ff137 commented Jul 25, 2024 •

edited

Loading

ff137 commented Jul 26, 2024 •

edited

Loading

ff137 commented Aug 5, 2024 •

edited

Loading

ff137 commented Aug 16, 2024 •

edited

Loading