Updated Backend Client #49
Conversation
WalkthroughThis pull request introduces significant enhancements to the OneLens backend client, focusing on expanding tenant management, metrics control, and embedding services. The changes primarily involve adding new classes, methods, and fields across multiple files in the Changes
Sequence DiagramsequenceDiagram
participant Client
participant NaviraEmbeddingService
participant MetricsControlService
participant TenantPolicyService
Client->>NaviraEmbeddingService: update_embeddings()
NaviraEmbeddingService-->>Client: Embedding Update Response
Client->>MetricsControlService: refresh_change_manager_status()
MetricsControlService-->>Client: Status Refresh Response
Client->>TenantPolicyService: search_tenant_policies()
TenantPolicyService-->>Client: Tenant Policies
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
onelens_backend_client_v2/rpc/tenant_policy_service_rpc_handler.py (2)
110-113: Remove extra blank lines between imports.The imports are correctly added but contain unnecessary blank lines between them. Consider removing the extra blank lines to maintain consistency with other imports in the file.
from onelens_backend_client_v2.models import SearchTenantPoliciesRequest - - from onelens_backend_client_v2.models import SearchTenantPoliciesResponse
2190-2254: Remove extra blank lines in docstring.The method implementation looks good and follows the established patterns, but there are unnecessary blank lines in the docstring between the description and parameters.
"""Search Tenant Policies - - - :param request: (required)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
onelens_backend_client_v2/models.py(28 hunks)onelens_backend_client_v2/rpc/__init__.py(2 hunks)onelens_backend_client_v2/rpc/cloud_metadata_service_rpc_handler.py(2 hunks)onelens_backend_client_v2/rpc/metrics_control_service_rpc_handler.py(4 hunks)onelens_backend_client_v2/rpc/navira_embedding_service_rpc_handler.py(1 hunks)onelens_backend_client_v2/rpc/tenant_policy_service_rpc_handler.py(2 hunks)
🔇 Additional comments (24)
onelens_backend_client_v2/models.py (16)
1078-1080: LGTM: New enum class for embedding types
The EmbeddingType enum class is well-defined with a clear purpose for handling different types of embeddings, currently supporting "TAGS".
1246-1246: LGTM: Optional tenant_id field addition
The optional tenant_id field of type UUID4 is appropriately added to the GetCloudAccountMetadataRequest class.
1307-1313: LGTM: New response model for metrics control webhook
The GetMetricsControlWebhookResponse class is well-structured with a required status field that describes the change manager request webhook status.
1512-1516: LGTM: New response model for tags
The GetTagsResponse class is well-defined with a required tags field that holds a dictionary of tag information.
1900-1909: LGTM: New model for Navira logs
The NaviraLog class is well-structured with all required fields for tracking query execution details:
query_id,user_idfor identificationusername,queryfor user contextstatus,sql_query_translationfor execution detailscreated_at,updated_atfor timestamps
2133-2145: LGTM: New pagination request models
The PostNaviraLogsRequest and PostTenantUsersRequest classes are well-defined with optional pagination parameters defaulting to page 1 with 10 items per page.
2500-2515: LGTM: New request/response models for change manager status
The RefreshChangeManagerStatusRequest and RefreshChangeManagerStatusResponse classes are well-structured for handling change manager status updates:
- Request includes optional list of event IDs
- Response includes required status field
3055-3081: LGTM: New response model for tenant details
The TenantDetailResponse class is well-structured with comprehensive fields for tenant information:
- Basic details: ID, name, status
- Subscription and account info
- Financial metrics: potential savings
- Ticket stats: open/closed counts
- Feature flags: hierarchy_exists
3223-3227: LGTM: New model for policy search results
The TenantPolicySearchResult class is well-defined with essential fields for policy search results:
id: UUID4 for unique identificationtitle: String for displayalias: String for reference
3350-3376: LGTM: New model for tenant user details
The TenantUserDetails class is well-structured with comprehensive user information:
- Personal info: name, email, contact details
- Professional info: role, job title, manager
- Preferences: language, currency, timezone
- System info: status, sources, timestamps
- Access control: node_ids
3564-3571: LGTM: New request model for embeddings update
The UpdateEmbeddingsRequest class is well-defined with required fields:
tenant_id: UUID4 for tenant identificationembedding_type: EmbeddingType enum for type specification
4315-4319: LGTM: Optional field addition for change manager events
The optional change_manager_event_ids field is appropriately added to the APIMetricsControlFilter class.
6036-6040: LGTM: Consistent field addition across models
The optional change_manager_event_ids field is consistently added to both GetMetricsControlStatusCountRequest and GetMetricsControlStatusRequest classes.
Also applies to: 6076-6080
Line range hint 6960-6988: LGTM: New response model for metrics control
The MetricsControlResponse class is well-structured with comprehensive fields for metrics control:
- Identification: id, resource_id
- Metrics info: metric_name, status
- Timestamps: requested_on, expires_on
- Additional data: entity_id, attributes
- Optional change manager event ID
9581-9606: LGTM: New request model for policy tickets notification
The GetPolicyTicketsForNotificationRequest class is well-defined with:
- Required tenant_id
- Optional filter criteria
- Boolean flags for controlling response data
10353-10356: LGTM: New response model for tenant policy search
The ResponseSearchTenantPoliciesResponse class follows the standard response pattern with data and optional message fields.
onelens_backend_client_v2/rpc/__init__.py (1)
105-107: LGTM! Import and export of NaviraEmbeddingServiceRpcHandler are properly implemented.
The changes follow the established pattern in the module for importing and exporting RPC handlers.
Also applies to: 181-181
onelens_backend_client_v2/rpc/cloud_metadata_service_rpc_handler.py (2)
14-18: LGTM! Import statements are properly organized.
The imports for the new tags-related models are correctly placed and follow the module's import style.
152-271: LGTM! The get_tags implementation follows the established pattern.
The method implementation is consistent with other RPC handlers in the class, including:
- Proper type annotations
- Complete error handling
- Comprehensive documentation
- Consistent parameter serialization
onelens_backend_client_v2/rpc/metrics_control_service_rpc_handler.py (3)
44-47: LGTM! Import statements for RefreshChangeManagerStatus models are properly placed.
The imports follow the module's organization pattern.
687-687: LGTM! Return type specification is more precise.
The change from Any to GetMetricsControlWebhookResponse improves type safety and makes the API contract clearer.
Also applies to: 725-725
793-912: LGTM! The refresh_change_manager_status implementation is well-structured.
The new method follows the established pattern with:
- Proper type annotations
- Complete error handling
- Comprehensive documentation
- Consistent parameter serialization
onelens_backend_client_v2/rpc/tenant_policy_service_rpc_handler.py (2)
2255-2309: LGTM!
The serialization method is well-implemented and follows the established patterns in the codebase. It properly handles:
- Request body serialization
- HTTP headers configuration
- Authentication settings
- Resource path construction
2190-2309: Verify the usage of the new search functionality.
Let's verify if there are any existing components that should be updated to use this new search functionality.
| # authentication setting | ||
| _auth_settings: List[str] = [] | ||
|
|
There was a problem hiding this comment.
Ensure authentication settings are correctly configured
The _auth_settings variable is set to an empty list, which means no authentication methods are being applied. If the API endpoint requires authentication, you should specify the appropriate authentication methods here to ensure secure access.
| _response_types_map: Dict[str, Optional[str]] = { | ||
| "200": "Any", | ||
| "422": "HTTPValidationError", | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle unexpected HTTP status codes
The _response_types_map currently handles only status codes 200 and 422. It's important to handle other possible HTTP responses such as 400 (Bad Request), 401 (Unauthorized), 403 (Forbidden), 404 (Not Found), and 500 (Internal Server Error) to ensure robust error handling.
Consider updating the _response_types_map to include these status codes:
_response_types_map: Dict[str, Optional[str]] = {
"200": "Any",
"422": "HTTPValidationError",
+ "400": "HTTPBadRequestError",
+ "401": "HTTPUnauthorizedError",
+ "403": "HTTPForbiddenError",
+ "404": "HTTPNotFoundError",
+ "500": "HTTPServerError",
}Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
Bug Fixes
Chores