[BREAKING CHANGE] Handle replies including without mention#3
Open
[BREAKING CHANGE] Handle replies including without mention#3
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces breaking changes to how reply-related tasks are handled, requiring a migration of the current Redis queue. The key changes include:
- Updating the Task structure to replace "post_cid" and "post_uri" with "cid", "parent_uri", and a new "root_uri".
- Modifying functions in Scheduler, MentionListener, ErrorHandler, and AtClient to accommodate the new reply handling mechanism.
- Adjusting type annotations and return values (e.g., in parse_create_op) to support the updated logic.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scheduler.py | Updated Task fields and post_reply call to include cid, parent_uri, and root_uri. |
| mention_listener.py | Changed parse_create_op return types and enqueue_reminder parameters for reply handling. |
| error_handler.py | Revised error handler signatures and post_reply usage to incorporate root_uri. |
| at_client.py | Modified post_reply and build_mention_post to support the new reply handling parameters. |
Comments suppressed due to low confidence (3)
scheduler.py:29
- [nitpick] The parameter 'cid' is used without a clear indication if it represents the parent ID or the reply's ID. Consider renaming it to 'parent_cid' if that better reflects its role.
self.at_client.post_reply(post, cid, parent_uri, root_uri)
mention_listener.py:55
- Ensure that test cases are updated or added to validate the new 'root_uri' parameter, verifying that reminder scheduling behaves as expected.
def enqueue_reminder(self, did: str, run_at: datetime, cid: str, parent_uri: str, root_uri: str) -> None:
error_handler.py:13
- [nitpick] The warning message only references the 'parent_uri'. Consider incorporating 'root_uri' into the error log to provide complete context when no run time is parsed.
def handle_no_run_at(self, did: str, parent_cid: str, parent_uri: str, root_uri: str) -> None:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a breaking change and will require migrating over the current redis queue to be compatible.