Skip to content

Feature: SQLite-based Message Cache #31

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

Closed
wants to merge 34 commits into from
Closed

Conversation

MHHukiewitz
Copy link
Member

Problem

Some apps require access to the same messages over and over again. This puts unnecessary strain on the CCN API and increases the latency of apps using Aleph Messages.

Solution

This pull request proposes a solution by introducing a message cache implemented with Peewee and an SQLite database in the Aleph SDK. The message cache effectively stores frequently accessed messages, eliminating the need for repetitive CCN API calls. With the message cache in place, apps utilizing the Aleph SDK will experience enhanced efficiency and responsiveness in accessing messages.

@hoh hoh self-requested a review May 24, 2023 15:03
@MHHukiewitz
Copy link
Member Author

MHHukiewitz commented May 24, 2023

After introducing the AlephClientInterface, I get a mypy error, that I really don't get: https://github.com/aleph-im/aleph-sdk-python/actions/runs/5071082316/jobs/9107016057?pr=31

Even though I am exactly copying the signature of the watch_messages function:

    async def watch_messages(
        self,
        message_type: Optional[MessageType] = None,
        content_types: Optional[Iterable[str]] = None,
        refs: Optional[Iterable[str]] = None,
        addresses: Optional[Iterable[str]] = None,
        tags: Optional[Iterable[str]] = None,
        hashes: Optional[Iterable[str]] = None,
        channels: Optional[Iterable[str]] = None,
        chains: Optional[Iterable[str]] = None,
        start_date: Optional[Union[datetime, float]] = None,
        end_date: Optional[Union[datetime, float]] = None,
    ) -> AsyncIterable[AlephMessage]:

mypy raises:

src/aleph/sdk/cache.py:370:5: error: Return type "AsyncIterable[Any]" of "watch_messages" incompatible with return type "Coroutine[Any, Any, AsyncIterable[Any]]" in supertype "AlephClientInterface" [override]
src/aleph/sdk/client.py:708:5: error: Return type "AsyncIterable[Any]" of "watch_messages" incompatible with return type "Coroutine[Any, Any, AsyncIterable[Any]]" in supertype "AlephClientInterface" [override]

@MHHukiewitz MHHukiewitz marked this pull request as ready for review June 6, 2023 14:35
@MHHukiewitz
Copy link
Member Author

Looking into snapshot testing to figure out how to mock API responses.

Copy link
Member

@hoh hoh left a comment

Choose a reason for hiding this comment

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

I added a first batch of comments.

@MHHukiewitz
Copy link
Member Author

Thanks @hoh, this helps prioritizing on which points work on next for me.

@MHHukiewitz MHHukiewitz force-pushed the mhh-sqlite-cache branch 3 times, most recently from 9d91846 to af853c5 Compare August 12, 2023 14:26
@MHHukiewitz
Copy link
Member Author

@hoh
I'd like to point out that this line:

if isinstance(messages, AlephMessage):

is making tests fail in Python versions =< 3.9. The alternative is making comparisons with every single Aleph message type (PostMessage, AggregateMessage, ...) which I think might be a bit ridiculous.

As Python progressed already to 3.11 now, can it be expected of people wanting to use our newest SDK versions to use at least Python 3.10?

@MHHukiewitz MHHukiewitz requested a review from hoh August 15, 2023 15:02
@hoh
Copy link
Member

hoh commented Aug 16, 2023

@hoh I'd like to point out that this line:

if isinstance(messages, AlephMessage):

is making tests fail in Python versions =< 3.9. The alternative is making comparisons with every single Aleph message type (PostMessage, AggregateMessage, ...) which I think might be a bit ridiculous.

The solution seems to be using isinstance(data, typing.get_args(Constant))

https://stackoverflow.com/a/73874277

As Python progressed already to 3.11 now, can it be expected of people wanting to use our newest SDK versions to use at least Python 3.10?

Not as long as we support Debian 11, which ships with Python 3.9.
The SDK should work on as many environments as possible, and keep doing so for a while. The situation is different for PyAleph since it is only expected to be deployed in containers and already uses Python 3.11.

@MHHukiewitz
Copy link
Member Author

So, resolved code duplication and other issues as pointed out by @hoh.

I made some breaking changes on the interface of the Client, which should have been done anyways (like changing the return type of get_posts() to a PostsResponse type, similar to get_messages() -> MessagesResponse. I hope this does not impact the review, splitting those changes in distinct PRs would be a chore. I would be OK with reviewing the changes together, if needed.

After extensive testing, whereby we are now reaching 61% test coverage, I am convinced that this branch is reliable and can be publicized in a future minor release.

@MHHukiewitz
Copy link
Member Author

Closed for now, as it will be broken down into smaller PRs

@MHHukiewitz MHHukiewitz deleted the mhh-sqlite-cache branch September 25, 2023 17:33
@MHHukiewitz MHHukiewitz restored the mhh-sqlite-cache branch September 25, 2023 17:33
@MHHukiewitz MHHukiewitz deleted the mhh-sqlite-cache branch November 8, 2023 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants