Revise message broker interface #3986
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size |
| @@ -1,31 +1,34 @@ | |||
| import abc | |||
| import importlib | |||
| import inspect | |||
| from typing import Any, Callable, Optional, Tuple, Dict | |||
There was a problem hiding this comment.
Everything in this file is from running black. Sorry!
|
I'm assuming this is related to #3831 |
| class MessageMetadata: | ||
| """Struct for metadata about a message.""" | ||
|
|
||
| sender: Sender |
There was a problem hiding this comment.
i like this approach to cohesively have the sender in the meta data
| sender: Sender | ||
| timestamp: datetime | ||
| additional_metadata: Dict = None | ||
| # TODO: what else goes here? |
There was a problem hiding this comment.
maybe commands and arguments ?
There was a problem hiding this comment.
Messages won't always have commands, I think. The additional_metadata dict gives us a lot of flexibility for now until you and @merwanehamadi and others can pull together the data interchange standard
|
|
||
| content: dict # Some json struct we can define with a strict schema |
There was a problem hiding this comment.
Would this be the msg broker contract we have talked about in the past?
There was a problem hiding this comment.
Where in the code should this structure be defined ?
There was a problem hiding this comment.
The content and the metadata.
They should get define in this subpackage, but I'm not sure what tool you want to use. There are lots of good ones, though pydantic might be industry standard at this point. I'm agnostic as long as the schema is enforcable.
autogpt/core/messaging/base.py
Outdated
| ... | ||
|
|
||
| @abc.abstractmethod | ||
| def send_message(self, content: dict, **extra_metadata) -> None: |
There was a problem hiding this comment.
should there be a return type to confirm that the message was sent? Maybe a boolean?
There was a problem hiding this comment.
Good Q. I'm not sure if the send receipt would be implementation specific. We can readjust when we do our first real message broker implementation
There was a problem hiding this comment.
I'll swap to a bool for now.
|
This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size |
collijk
merged commit b2ffa0a
into
Significant-Gravitas:agent-state-encapsulation
Updates the abstract interface for the message broker as it starts to meet the real world in #3969