feat: enhance onelens backend client with new RPC handlers and models…#82
feat: enhance onelens backend client with new RPC handlers and models…#82ramanjangu1 wants to merge 0 commit into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Onelens backend client by integrating a suite of new RPC handlers and models. These additions provide robust support for managing aggregated interactions, visualizing savings data through a dedicated dashboard, handling violations, and offering more granular control over custom policies and tickets. The changes streamline various backend operations and expand the client's ability to interact with core platform services. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request enhances the onelens backend client by adding several new RPC handlers and models for features like aggregated interactions, savings dashboards, and violations. The changes are mostly auto-generated boilerplate. My review focuses on improving code maintainability by organizing imports and suggesting the use of aliases for excessively long, auto-generated model names to improve readability.
Note: Security Review did not run due to the size of the PR.
| from onelens_backend_client_v2.rpc.aggregated_savings_dashboard_service_rpc_handler import ( | ||
| AggregatedSavingsDashboardServiceRpcHandler, | ||
| ) | ||
|
|
||
| from onelens_backend_client_v2.rpc.aggregated_violations_service_rpc_handler import ( | ||
| AggregatedViolationsServiceRpcHandler, | ||
| ) | ||
|
|
||
| from onelens_backend_client_v2.rpc.aggregated_interactions_service_rpc_handler import ( | ||
| AggregatedInteractionsServiceRpcHandler, | ||
| ) | ||
|
|
||
| from onelens_backend_client_v2.rpc.custom_tickets_service_rpc_handler import ( | ||
| CustomTicketsServiceRpcHandler, | ||
| ) | ||
|
|
||
| from onelens_backend_client_v2.rpc.custom_policy_service_rpc_handler import ( | ||
| CustomPolicyServiceRpcHandler, | ||
| ) | ||
|
|
||
| from onelens_backend_client_v2.rpc.custom_ticket_history_service_rpc_handler import ( | ||
| CustomTicketHistoryServiceRpcHandler, | ||
| ) | ||
|
|
||
| from onelens_backend_client_v2.rpc.hierarchy_node_service_v2_rpc_handler import ( | ||
| HierarchyNodeServiceV2RpcHandler, | ||
| ) |
There was a problem hiding this comment.
For better maintainability and adherence to PEP 8, it's recommended to group related imports and sort them alphabetically. The new imports can be combined and ordered.
from onelens_backend_client_v2.rpc.aggregated_interactions_service_rpc_handler import (
AggregatedInteractionsServiceRpcHandler,
)
from onelens_backend_client_v2.rpc.aggregated_savings_dashboard_service_rpc_handler import (
AggregatedSavingsDashboardServiceRpcHandler,
)
from onelens_backend_client_v2.rpc.aggregated_violations_service_rpc_handler import (
AggregatedViolationsServiceRpcHandler,
)
from onelens_backend_client_v2.rpc.custom_policy_service_rpc_handler import (
CustomPolicyServiceRpcHandler,
)
from onelens_backend_client_v2.rpc.custom_ticket_history_service_rpc_handler import (
CustomTicketHistoryServiceRpcHandler,
)
from onelens_backend_client_v2.rpc.custom_tickets_service_rpc_handler import (
CustomTicketsServiceRpcHandler,
)
from onelens_backend_client_v2.rpc.hierarchy_node_service_v2_rpc_handler import (
HierarchyNodeServiceV2RpcHandler,
)| "AggregatedSavingsDashboardServiceRpcHandler", | ||
| "AggregatedViolationsServiceRpcHandler", | ||
| "AggregatedInteractionsServiceRpcHandler", | ||
| "CustomTicketsServiceRpcHandler", | ||
| "CustomPolicyServiceRpcHandler", | ||
| "CustomTicketHistoryServiceRpcHandler", | ||
| "HierarchyNodeServiceV2RpcHandler", |
There was a problem hiding this comment.
To improve code readability and maintainability, the __all__ list should be sorted alphabetically. Please sort the newly added entries.
"AggregatedInteractionsServiceRpcHandler",
"AggregatedSavingsDashboardServiceRpcHandler",
"AggregatedViolationsServiceRpcHandler",
"CustomPolicyServiceRpcHandler",
"CustomTicketHistoryServiceRpcHandler",
"CustomTicketsServiceRpcHandler",
"HierarchyNodeServiceV2RpcHandler",| from onelens_backend_client_v2.models import GetAggregatedPolicyStatsResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedPolicyFiltersRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedPolicyFiltersResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import UpsertAggregatedPoliciesRequest | ||
|
|
||
|
|
There was a problem hiding this comment.
To improve readability and follow Python best practices, it's better to group imports from the same module. Please group these imports into a single block and sort them alphabetically.
| from onelens_backend_client_v2.models import GetAggregatedPolicyStatsResponse | |
| from onelens_backend_client_v2.models import GetAggregatedPolicyFiltersRequest | |
| from onelens_backend_client_v2.models import GetAggregatedPolicyFiltersResponse | |
| from onelens_backend_client_v2.models import UpsertAggregatedPoliciesRequest | |
| from onelens_backend_client_v2.models import ( | |
| GetAggregatedPolicyFiltersRequest, | |
| GetAggregatedPolicyFiltersResponse, | |
| GetAggregatedPolicyStatsResponse, | |
| UpsertAggregatedPoliciesRequest, | |
| ) |
| from onelens_backend_client_v2.models import DeleteAggregatedTicketsResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedTicketFiltersRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedTicketFiltersResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedTicketsStatsRequest | ||
|
|
||
|
|
There was a problem hiding this comment.
For better readability and adherence to PEP 8, please group imports from the same module and sort them alphabetically.
| from onelens_backend_client_v2.models import DeleteAggregatedTicketsResponse | |
| from onelens_backend_client_v2.models import GetAggregatedTicketFiltersRequest | |
| from onelens_backend_client_v2.models import GetAggregatedTicketFiltersResponse | |
| from onelens_backend_client_v2.models import GetAggregatedTicketsStatsRequest | |
| from onelens_backend_client_v2.models import ( | |
| DeleteAggregatedTicketsRequest, | |
| DeleteAggregatedTicketsResponse, | |
| GetAggregatedTicketFiltersRequest, | |
| GetAggregatedTicketFiltersResponse, | |
| GetAggregatedTicketsStatsRequest, | |
| GetAggregatedTicketsStatsResponse, | |
| ) |
| from onelens_backend_client_v2.models import GetAggregatedTicketsAPIResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedTicketsExportRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedTicketsExportResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedTicketsForExportRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetAggregatedTicketsForExportResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetTicketUsersRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetTicketUsersResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import UpsertAggregatedTicketDimensionsRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import UpsertAggregatedTicketDimensionsResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import UpsertAggregatedTicketsRequest | ||
|
|
||
|
|
There was a problem hiding this comment.
These imports can be grouped into a single block and sorted alphabetically to improve code clarity and maintainability.
| from onelens_backend_client_v2.models import GetAggregatedTicketsAPIResponse | |
| from onelens_backend_client_v2.models import GetAggregatedTicketsExportRequest | |
| from onelens_backend_client_v2.models import GetAggregatedTicketsExportResponse | |
| from onelens_backend_client_v2.models import GetAggregatedTicketsForExportRequest | |
| from onelens_backend_client_v2.models import GetAggregatedTicketsForExportResponse | |
| from onelens_backend_client_v2.models import GetTicketUsersRequest | |
| from onelens_backend_client_v2.models import GetTicketUsersResponse | |
| from onelens_backend_client_v2.models import UpsertAggregatedTicketDimensionsRequest | |
| from onelens_backend_client_v2.models import UpsertAggregatedTicketDimensionsResponse | |
| from onelens_backend_client_v2.models import UpsertAggregatedTicketsRequest | |
| from onelens_backend_client_v2.models import ( | |
| GetAggregatedTicketsAPIResponse, | |
| GetAggregatedTicketsExportRequest, | |
| GetAggregatedTicketsExportResponse, | |
| GetAggregatedTicketsForExportRequest, | |
| GetAggregatedTicketsForExportResponse, | |
| GetTicketUsersRequest, | |
| GetTicketUsersResponse, | |
| UpsertAggregatedTicketDimensionsRequest, | |
| UpsertAggregatedTicketDimensionsResponse, | |
| UpsertAggregatedTicketsRequest, | |
| UpsertAggregatedTicketsResponse, | |
| ) |
| from onelens_backend_client_v2.models import GetCURDataQueryResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetResourceDetailsRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetResourceDetailsResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import GetResourceDetailsFromARNForCURRequest | ||
|
|
||
|
|
There was a problem hiding this comment.
To improve readability and maintainability, please group these imports from the same module into a single block and sort them alphabetically.
| from onelens_backend_client_v2.models import GetCURDataQueryResponse | |
| from onelens_backend_client_v2.models import GetResourceDetailsRequest | |
| from onelens_backend_client_v2.models import GetResourceDetailsResponse | |
| from onelens_backend_client_v2.models import GetResourceDetailsFromARNForCURRequest | |
| from onelens_backend_client_v2.models import ( | |
| GetCURDataQueryResponse, | |
| GetResourceDetailsFromARNForCURRequest, | |
| GetResourceDetailsFromARNForCURResponse, | |
| GetResourceDetailsRequest, | |
| GetResourceDetailsResponse, | |
| ) |
| from onelens_backend_client_v2.models import ( | ||
| onelens__models__service_interfaces__tenant_metadata__hierarchy_node_dto__CreateHierarchyNodeRequest, | ||
| ) | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import CreateHierarchyNodeResponse | ||
| from onelens_backend_client_v2.models import ( | ||
| onelens__models__service_interfaces__tenant_metadata__hierarchy_node_dto__CreateHierarchyNodeResponse, | ||
| ) |
There was a problem hiding this comment.
The new model names are very long and harm readability. Consider using aliases for these long model names to improve code clarity. For example:
from onelens_backend_client_v2.models import (
onelens__models__service_interfaces__tenant_metadata__hierarchy_node_dto__CreateHierarchyNodeRequest as CreateHierarchyNodeRequest,
)This would allow you to use the shorter CreateHierarchyNodeRequest in method signatures, making the code much easier to read and maintain. This pattern should be applied consistently for all long model names in this file.
| from onelens_backend_client_v2.models import OnboardGCPTenantResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import OnboardGitHubTenantRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import OnboardGitHubTenantResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import OnboardOCITenantRequest | ||
|
|
||
|
|
There was a problem hiding this comment.
To improve readability and follow Python best practices, it's better to group imports from the same module. Please group these imports into a single block and sort them alphabetically.
| from onelens_backend_client_v2.models import OnboardGCPTenantResponse | |
| from onelens_backend_client_v2.models import OnboardGitHubTenantRequest | |
| from onelens_backend_client_v2.models import OnboardGitHubTenantResponse | |
| from onelens_backend_client_v2.models import OnboardOCITenantRequest | |
| from onelens_backend_client_v2.models import ( | |
| OnboardGCPTenantResponse, | |
| OnboardGitHubTenantRequest, | |
| OnboardGitHubTenantResponse, | |
| OnboardOCITenantRequest, | |
| ) |
| from onelens_backend_client_v2.models import BulkUpdateTenantTicketsResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import ComprehensiveUpdateTenantTicketRequest | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import ComprehensiveUpdateTenantTicketResponse | ||
|
|
||
|
|
||
| from onelens_backend_client_v2.models import CreateCustomTenantTicketsRequest | ||
|
|
||
|
|
There was a problem hiding this comment.
For better readability and adherence to PEP 8, please group imports from the same module and sort them alphabetically.
| from onelens_backend_client_v2.models import BulkUpdateTenantTicketsResponse | |
| from onelens_backend_client_v2.models import ComprehensiveUpdateTenantTicketRequest | |
| from onelens_backend_client_v2.models import ComprehensiveUpdateTenantTicketResponse | |
| from onelens_backend_client_v2.models import CreateCustomTenantTicketsRequest | |
| from onelens_backend_client_v2.models import ( | |
| BulkUpdateTenantTicketsRequest, | |
| BulkUpdateTenantTicketsResponse, | |
| ComprehensiveUpdateTenantTicketRequest, | |
| ComprehensiveUpdateTenantTicketResponse, | |
| CreateCustomTenantTicketsRequest, | |
| ) |
| from onelens_backend_client_v2.models import ( | ||
| onelens__models__service_interfaces__tickets__tenant_ticket_service_dto__GetCustomTicketDetailsRequest, | ||
| ) |
There was a problem hiding this comment.
This model name is excessively long and harms readability. Please use an alias to shorten it. This will make the code in this file much easier to read and maintain. After aliasing, you can use the shorter name in method signatures.
For example:
from onelens_backend_client_v2.models import (
onelens__models__service_interfaces__tickets__tenant_ticket_service_dto__GetCustomTicketDetailsRequest as GetCustomTicketDetailsRequest,
)1a6ac73 to
68e8b3e
Compare
68e8b3e to
384d638
Compare
… for aggregated interactions, savings dashboard, and violations