8000 Feature: get_message_status and overload of get_message to return status by 1yam · Pull Request #163 · aleph-im/aleph-sdk-python · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feature: get_message_status and overload of get_message to return status #163

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

Jump to bottom
Merged
merged 3 commits into from
Sep 3, 2024
Merged

Feature: get_message_status and overload of get_message to return status #163

merged 3 commits into from
Sep 3, 2024

Conversation

1yam
Copy link
Member
@1yam 1yam commented Aug 30, 2024 •
edited
Loading

Problem: Currently, there is no method to retrieve the status of a message using its item_hash. This limitation makes it difficult to efficiently check the processing state of specific messages.

Solution: I have added a new method, get_message_status, which allows users to obtain the status of a message by providing the item_hash. I have updated the get_message method to optionally return the status of a message when requested, improving overall flexibility and functionality.

@github-actions GitHub Actions
Copy link

Summary and Analysis

The PR introduces significant changes to the http.py file, including:

  1. Imports and Type Annotations: The file now includes numerous new imports and complex type annotations, which could potentially introduce bugs or misunderstandings.
  2. Overloads: The addition of @overload declarations for get_message and get_message_status methods introduces complexity, potentially affecting the readability and maintainability of the code.
  3. Status Handling: The introduction of with_status parameter and handling of status responses adds additional complexity and could affect the overall stability of the function.

These changes are likely to have a high potential for introducing bugs or requiring deep understanding of the project architecture, making them suitable for a 'BLACK' level of review.

@github-actions github-actions bot added the BLACK This PR has critical implications and must be reviewed by a senior engineer. label Aug 30, 2024
philogicae
philogicae previously approved these changes Aug 30, 2024
View reviewed changes
Copy link
Member
@philogicae philogicae left a comment

Choose a reason for hiding this comment

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

LGTM

@1yam 1yam mentioned this pull request Aug 30, 2024
Merged
5 tasks
hoh
hoh reviewed Sep 3, 2024
View reviewed changes
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.

No test on this ?

philogicae
philogicae approved these changes Sep 3, 2024
View reviewed changes
Copy link
Member
@philogicae philogicae left a comment

Choose a reason for hiding this comment

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

Reviewed, fixed and ready

@philogicae philogicae merged commit c62fbb7 into main Sep 3, 2024
13 checks passed
@philogicae philogicae deleted the 1yam-message-status branch September 3, 2024 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoh hoh hoh left review comments

@philogicae philogicae philogicae approved these changes

Assignees

@philogicae philogicae

Labels
BLACK This PR has critical implications and must be reviewed by a senior engineer.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@1yam @hoh @philogicae
0