-
Notifications
You must be signed in to change notification settings - Fork 42
Internal utilities and wrappers for Storage protocols #201
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces storage wrapper classes to improve the modularity and organization of storage operations in the microsoft-agents framework. The changes include new wrapper implementations for namespaced and item-based storage access patterns, along with corresponding test coverage.
Key changes:
- Added
_StorageNamespace,_ItemStorage, and_ItemNamespacewrapper classes to provide namespace-based key isolation and single-item storage operations - Refactored authorization code to use the new storage wrappers instead of direct storage access
- Updated imports and type hints across the codebase for consistency
- Enhanced error handling by replacing
passwithraise NotImplementedError()in abstract methods
Reviewed Changes
Copilot reviewed 28 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/hosting_core/storage/wrappers/test_storage_namespace.py |
Comprehensive test suite for _StorageNamespace wrapper |
tests/hosting_core/storage/wrappers/test_item_storage.py |
Test coverage for single-item storage operations |
tests/hosting_core/storage/wrappers/test_item_namespace.py |
Tests for combined namespace and item storage functionality |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/_wrappers/_storage_namespace.py |
Implementation of namespace-based storage key prefixing |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/_wrappers/_item_storage.py |
Single-item storage operation wrapper |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/_wrappers/_item_namespace.py |
Combines namespace and item storage patterns |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/storage.py |
Updated abstract methods to raise NotImplementedError instead of pass; renamed AsyncStorageBase to _AsyncStorageBase |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/memory_storage.py |
Fixed scoping issue by moving result dictionary initialization |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/authorization.py |
Refactored to use new storage wrappers for sign-in state management |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py |
Updated to use _ItemNamespace instead of deleted _FlowStorageClient |
libraries/microsoft-agents-activity/microsoft_agents/activity/_utils/_error_handling.py |
New error handling utilities for parameter validation |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/storage/_namespaces.py |
Centralized storage namespace constants |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| TranscriptMemoryStore, | ||
| ) | ||
| from ._wrappers import ( | ||
| _StorageNamespace |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comma after '_StorageNamespace' on line 17 causes a syntax error when followed by '_ItemNamespace' on line 18.
| _StorageNamespace | |
| _StorageNamespace, |
| "aud" | ||
| ] | ||
|
|
||
| namespace = _Namespaces._USER_AUTHORIZATION.format( |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference to '_Namespaces._USER_AUTHORIZATION' is incorrect. The attribute in _namespaces.py is defined as 'USER_AUTHORIZATION' (without underscore prefix), so this should be '_Namespaces.USER_AUTHORIZATION'.
| namespace = _Namespaces._USER_AUTHORIZATION.format( | |
| namespace = _Namespaces.USER_AUTHORIZATION.format( |
|
|
||
| namespace = _Namespaces._USER_AUTHORIZATION.format( | ||
| channel_id=channel_id, | ||
| user_id=user_id, |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format string expects 'from_property_id' as a parameter (line 8 in _namespaces.py), but 'user_id' is being passed here. This will cause a KeyError when attempting to format the namespace string.
This pull request refactors and improves the storage and error handling infrastructure for the Microsoft Agents activity and hosting core libraries. The main changes involve modularizing utility functions, introducing new storage namespace abstractions, and replacing custom storage client logic with standardized wrappers for managing state. These updates simplify code maintenance, enhance clarity, and make state management more robust and consistent.
Error handling and utility improvements:
_raise_if_noneand_raise_if_falseyhelper functions for input validation, and movedload_configuration_from_envto a new_utilsmodule for better organization. [1] [2] [3] [4]Storage abstraction and namespace improvements:
_Namespacesand storage wrapper classes (_ItemNamespace,_StorageNamespace,_ItemStorage) to standardize namespacing and item management in storage operations. These are now exported in the storage package for use across the codebase. [1] [2]_ItemNamespacefor sign-in state management, replacing custom key generation and CRUD methods with standardized wrapper calls. [1] [2] [3] [4] [5] [6] [7] [8]Removal of legacy code and simplification:
_FlowStorageClientclass and its usage, replacing it with_ItemNamespaceand standardized storage wrappers for OAuth flow state management. [1] [2] [3] [4] [5] [6]Internal API consistency:
_StorageNamespacefor isolating storage per handler, improving separation of concerns and consistency.Documentation and code clarity: