Async user hooks #1845
Closed
Async user hooks #1845
Conversation
This file contains 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
added 2 commits
March 3, 2024 16:20
create async versions of hooks in converable agents, async chat will assume async hooks so event loop won't be mixed with sync/async contexts.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1845 +/- ##
==========================================
+ Coverage 36.03% 45.68% +9.65%
==========================================
Files 63 63
Lines 6608 6661 +53
Branches 1456 1600 +144
==========================================
+ Hits 2381 3043 +662
+ Misses 4036 3369 -667
- Partials 191 249 +58
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| # Message modifications do not affect the incoming messages or self._oai_messages. | ||
| messages = self.process_all_messages_before_reply(messages) | ||
| messages = await self.a_process_last_received_message(messages) |
There was a problem hiding this comment.
The ordering between the calling of process_all_messages_before_reply and process_last_received_message recently changed. Have you merged from main since then?
gagb
reviewed
Mar 8, 2024
| """Process the message before sending it to the recipient.""" | ||
| hook_list = self.hook_lists["a_process_message_before_send"] | ||
| for hook in hook_list: | ||
| if not inspect.iscoroutinefunction(hook): |
There was a problem hiding this comment.
Should we be throwing warning here?
| @@ -1106,6 +1106,30 @@ def send_to_frontend(sender, message, recipient, silent): | |||
| print_mock.assert_called_once_with(message="hello") | |||
|
|
|||
|
|
|||
| @pytest.mark.asyncio | |||
| async def test_process_before_send_async(): | |||
There was a problem hiding this comment.
test_a_process_before_send() for consistency?
| print_mock = unittest.mock.MagicMock() | ||
|
|
||
| # Updated to include sender parameter | ||
| async def a_send_to_frontend(sender, message, recipient, silent): |
There was a problem hiding this comment.
This is fantastic! I have the exact same use case!
sonichi
requested review from
yenif,
davorrunje,
rajan-chari,
bitnom,
cemtu and
RohitRathore1
cemtu
reviewed
Mar 11, 2024
| @@ -552,9 +555,22 @@ def _process_message_before_send( | |||
| """Process the message before sending it to the recipient.""" | |||
| hook_list = self.hook_lists["process_message_before_send"] | |||
| for hook in hook_list: | |||
| if inspect.iscoroutinefunction(hook): | |||
There was a problem hiding this comment.
What do you think about throwing a warning over here?
As far as I have seen, this is common pattern that we need to have it in each synchronous/async hook to check if the registered method matches the expected execution context, ensuring that asynchronous hooks are not mistakenly used in synchronous contexts and vice versa.
Can we have a more generalized logic to control such misplacements?
What do you think about updating the register_hook method a bit to support this?
def register_hook(self, hookable_method: Callable, hook: Callable):
"""
Registers a hook to be called by a hookable method, in order to add a capability to the agent.
Registered hooks are kept in lists (one per hookable method), and are called in their order of registration.
Args:
hookable_method: A hookable method implemented by ConversableAgent.
hook: A method implemented by a subclass of AgentCapability.
"""
assert hookable_method in self.hook_lists, f"{hookable_method} is not a hookable method."
hook_list = self.hook_lists[hookable_method]
assert hook not in hook_list, f"{hook} is already registered as a hook."
#### SUGGESTED CHANGES STARTS ####
expected_async = hookable_method.startswith('a_')
hook_is_async = inspect.iscoroutinefunction(hook)
if expected_async != hook_is_async:
context_type = "asynchronous" if expected_async else "synchronous"
warnings.warn(f"Hook '{hook.__name__}' is {'asynchronous' if hook_is_async else 'synchronous'}, "
f"but it's being registered in a {context_type} context ('{hookable_method}'). "
"Ensure the hook matches the expected execution context.", UserWarning)
#### SUGGESTED CHANGES ENDS ####
hook_list.append(hook)This suggested solution above, can also address the suggestion from @gagb https://github.com/microsoft/autogen/pull/1845/files#r1518417767.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 12853598 | Triggered | Generic High Entropy Secret | 79dbb7b | test/oai/test_utils.py | View secret |
| 10404693 | Triggered | Generic High Entropy Secret | e43a86c | test/oai/test_utils.py | View secret |
| 10404693 | Triggered | Generic High Entropy Secret | bdb40d7 | test/oai/test_utils.py | View secret |
| 10404693 | Triggered | Generic High Entropy Secret | 954ca45 | test/oai/test_utils.py | View secret |
| 10404693 | Triggered | Generic High Entropy Secret | 79dbb7b | test/oai/test_utils.py | View secret |
| 10404662 | Triggered | Generic CLI Secret | eff19ac | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | 06a0a5d | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | 0524c77 | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | d7ea410 | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | e43a86c | .github/workflows/dotnet-build.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | 841ed31 | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | 802f099 | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | 9a484d8 | .github/workflows/dotnet-build.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | e973ac3 | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | 89650e7 | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | e07b06b | .github/workflows/dotnet-release.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | abe4c41 | .github/workflows/dotnet-build.yml | View secret |
| 10404662 | Triggered | Generic CLI Secret | 7362fb9 | .github/workflows/dotnet-release.yml | View secret |
| 12853599 | Triggered | Generic High Entropy Secret | 79dbb7b | test/oai/test_utils.py | View secret |
| 10404694 | Triggered | Generic High Entropy Secret | e43a86c | test/oai/test_utils.py | View secret |
| 10404694 | Triggered | Generic High Entropy Secret | 954ca45 | test/oai/test_utils.py | View secret |
| 10404694 | Triggered | Generic High Entropy Secret | bdb40d7 | test/oai/test_utils.py | View secret |
| 10404695 | Triggered | Generic High Entropy Secret | abad9ff | test/oai/test_utils.py | View secret |
| 10404695 | Triggered | Generic High Entropy Secret | 954ca45 | test/oai/test_utils.py | View secret |
| 10404695 | Triggered | Generic High Entropy Secret | c7bb588 | test/oai/test_utils.py | View secret |
| 10404695 | Triggered | Generic High Entropy Secret | b97b99d | test/oai/test_utils.py | View secret |
| 10404695 | Triggered | Generic High Entropy Secret | e43a86c | test/oai/test_utils.py | View secret |
| 12853600 | Triggered | Generic High Entropy Secret | 79dbb7b | test/oai/test_utils.py | View secret |
| 12853601 | Triggered | Generic High Entropy Secret | 79dbb7b | test/oai/test_utils.py | View secret |
| 10493810 | Triggered | Generic Password | 49e8053 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 501610b | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 49e8053 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 501610b | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | d422c63 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 97fa339 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 49e8053 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | d422c63 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 97fa339 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | d422c63 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 97fa339 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 501610b | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10404696 | Triggered | Generic High Entropy Secret | 954ca45 | test/oai/test_utils.py | View secret |
| 10404696 | Triggered | Generic High Entropy Secret | bdb40d7 | test/oai/test_utils.py | View secret |
| 10404696 | Triggered | Generic High Entropy Secret | 79dbb7b | test/oai/test_utils.py | View secret |
| 10404696 | Triggered | Generic High Entropy Secret | e43a86c | test/oai/test_utils.py | View secret |
| 10422482 | Triggered | Generic High Entropy Secret | 79dbb7b | test/oai/test_utils.py | View secret |
| 10422482 | Triggered | Generic High Entropy Secret | bdb40d7 | test/oai/test_utils.py | View secret |
| 12853602 | Triggered | Generic High Entropy Secret | 79dbb7b | test/oai/test_utils.py | View secret |
| 11616921 | Triggered | Generic High Entropy Secret | a86d0fd | notebook/agentchat_agentops.ipynb | View secret |
| 11616921 | Triggered | Generic High Entropy Secret | 394561b | notebook/agentchat_agentops.ipynb | View secret |
| 11616921 | Triggered | Generic High Entropy Secret | 3eac646 | notebook/agentchat_agentops.ipynb | View secret |
| 11616921 | Triggered | Generic High Entropy Secret | f45b553 | notebook/agentchat_agentops.ipynb | View secret |
| 11616921 | Triggered | Generic High Entropy Secret | 6563248 | notebook/agentchat_agentops.ipynb | View secret |
| 12853598 | Triggered | Generic High Entropy Secret | 2b3a9ae | test/oai/test_utils.py | View secret |
| 12853598 | Triggered | Generic High Entropy Secret | c03558f | test/oai/test_utils.py | View secret |
| 10404693 | Triggered | Generic High Entropy Secret | c03558f | test/oai/test_utils.py | View secret |
| 10404693 | Triggered | Generic High Entropy Secret | 2b3a9ae | test/oai/test_utils.py | View secret |
| 10404693 | Triggered | Generic High Entropy Secret | 0a3c6c4 | test/oai/test_utils.py | View secret |
| 10404693 | Triggered | Generic High Entropy Secret | 76f5f5a | test/oai/test_utils.py | View secret |
| 10404662 | Triggered | Generic CLI Secret | 954ca45 | .github/workflows/dotnet-build.yml | View secret |
| 12853599 | Triggered | Generic High Entropy Secret | 2b3a9ae | test/oai/test_utils.py | View secret |
| 12853599 | Triggered | Generic High Entropy Secret | c03558f | test/oai/test_utils.py | View secret |
| 10404694 | Triggered | Generic High Entropy Secret | 76f5f5a | test/oai/test_utils.py | View secret |
| 10404694 | Triggered | Generic High Entropy Secret | 0a3c6c4 | test/oai/test_utils.py | View secret |
| 10404695 | Triggered | Generic High Entropy Secret | 3b79cc6 | test/oai/test_utils.py | View secret |
| 10404695 | Triggered | Generic High Entropy Secret | 11baa52 | test/oai/test_utils.py | View secret |
| 12853600 | Triggered | Generic High Entropy Secret | c03558f | test/oai/test_utils.py | View secret |
| 12853600 | Triggered | Generic High Entropy Secret | 2b3a9ae | test/oai/test_utils.py | View secret |
| 12853601 | Triggered | Generic High Entropy Secret | c03558f | test/oai/test_utils.py | View secret |
| 12853601 | Triggered | Generic High Entropy Secret | 2b3a9ae | test/oai/test_utils.py | View secret |
| 10493810 | Triggered | Generic Password | 3b79cc6 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 11baa52 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 11baa52 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10493810 | Triggered | Generic Password | 3b79cc6 | notebook/agentchat_pgvector_RetrieveChat.ipynb | View secret |
| 10404696 | Triggered | Generic High Entropy Secret | 0a3c6c4 | test/oai/test_utils.py | View secret |
| 10404696 | Triggered | Generic High Entropy Secret | 76f5f5a | test/oai/test_utils.py | View secret |
| 10404696 | Triggered | Generic High Entropy Secret | c03558f | test/oai/test_utils.py | View secret |
| 10404696 | Triggered | Generic High Entropy Secret | 2b3a9ae | test/oai/test_utils.py | View secret |
| 10422482 | Triggered | Generic High Entropy Secret | 2b3a9ae | test/oai/test_utils.py | View secret |
| 10422482 | Triggered | Generic High Entropy Secret | c03558f | test/oai/test_utils.py | View secret |
and 16 others.
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
@sidhujag it looks like this PR has stalled, do you have an interest or ability to complete? |
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.
Why are these changes needed?
Async user hooks create an async version of hooks to work with the event loop through async calls, not to get them confused with sync calls. Any async chats will use async hooks inherently so the user hooks can do async actions.
Checks