Skip to content

Conversation

@MHHukiewitz
Copy link
Member

Inspired by https://github.com/aleph-im/aleph-client/pull/129/files:

  • get_message now requires less parameters and raises ForgottenMessageError if message existed, but was forgotten
    • Uses the /api/v0/messages/{item_hash} endpoint to do so -> should potentially lead to performance improvements
  • The tests in test_asynchronous_get.py are now fully mocked
  • Refactored fixtures from test_asynchronous_get.py and test_asynchronous.py to be included in conftest.py

Solution: Mock responses; refactor to reduce duplication
…en forgotten

Solution: get_messages now calls .../messages/{item_hash} endpoint and raises ForgottenMessageException
@MHHukiewitz MHHukiewitz requested a review from BjrInt November 21, 2023 10:44
@github-actions github-actions bot added the RED This PR is complex and may require more time to review. label Nov 21, 2023
@github-actions
Copy link

The changes in this PR can be considered as having moderate risks due to the following reasons:

  • The addition of a new exception class ForgottenMessageError which extends the base QueryError.
  • Modifications in several functions like get_message, get_messages, and get_posts that involve changes in how they interact with the API, such as using async context managers and handling different response statuses.
  • Addition of new test cases for testing the retrieval of forgotten messages which may impact the system's behavior when dealing with forgotten messages.

The PR also includes some refactoring and documentation updates, but these changes are relatively minor and do not significantly increase the complexity of the review.

In summary, this PR requires a deeper understanding of the project architecture and potential risks involved in handling forgotten messages and different API response statuses. Therefore, it is categorized as 'RED'.

@BjrInt BjrInt self-requested a review November 21, 2023 14:10
@MHHukiewitz MHHukiewitz merged commit 59e22b5 into main Nov 22, 2023
@MHHukiewitz MHHukiewitz deleted the mhh-forgotten-message-exception branch November 22, 2023 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RED This PR is complex and may require more time to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants