WebOfTrust / keria

KERI Agent in the cloud
https://keria.readthedocs.io/en/latest/
Apache License 2.0
16 stars 26 forks source link

Add filters in notifications endpoint #267

Open rodolfomiranda opened 3 days ago

rodolfomiranda commented 3 days ago

This PR address issue #266 . It adds to NotificationCollectionEnd three optional query parameters

codecov[bot] commented 3 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.11%. Comparing base (18d3ad7) to head (50543d8). Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #267 +/- ## ========================================== + Coverage 93.06% 93.11% +0.04% ========================================== Files 36 36 Lines 7121 7243 +122 ========================================== + Hits 6627 6744 +117 - Misses 494 499 +5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lenkan commented 2 days ago

Very helpful feature!

Question: I am not familiar with the notifier implementation, but this looks like it would load all notes in to memory, then filter them? Would it make sense to instead implement the filtering in keripy, utilizing the iterator to avoid loading all notes into memory?

rodolfomiranda commented 2 days ago

That's a good point. The optimal way is to implement it in keripy but it needs to run the iterator also on the countAll function. However, for the order part we still need to get them all in memory before pagination. I can move it there easily, but I need the feedback from others specially @pfeairheller. If we are in agreement, I'll adapt this one and create a onother in keripy.