refactor-opt(torii-grpc): subscriptions no db fetch #2455
Conversation
WalkthroughOhayo, sensei! The changes in this pull request involve renaming fields in several structs to improve clarity, specifically changing Changes
Possibly related PRs
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (2)
Files skipped from review as they are similar to previous changes (2)
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
Documentation and Community
|
![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
Outside diff range and nitpick comments (2)
crates/torii/grpc/src/server/subscriptions/event_message.rs (1)
207-207: Consider handling errors more comprehensively in the spawned taskOhayo, sensei! Currently, if
Service::publish_updatesreturns an error, it's logged but not acted upon. Depending on the criticality of this operation, you might want to implement retry logic or propagate the error to ensure that subscribers consistently receive updates.crates/torii/grpc/src/server/subscriptions/entity.rs (1)
229-231: Ohayo, sensei! Consider enhancing error handling inpublish_updatesCurrently, if
publish_updatesreturns an error, it's logged but not otherwise handled. Depending on the criticality, you might want to implement a retry mechanism or propagate the error to ensure reliable operation.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- crates/torii/core/src/query_queue.rs (3 hunks)
- crates/torii/core/src/sql.rs (1 hunks)
- crates/torii/grpc/src/server/subscriptions/entity.rs (2 hunks)
- crates/torii/grpc/src/server/subscriptions/event_message.rs (2 hunks)
Files skipped from review due to trivial changes (1)
- crates/torii/core/src/sql.rs
Additional comments not posted (2)
crates/torii/core/src/query_queue.rs (2)
45-45: Ohayo sensei! The field rename looks good.The change from
entitytotybetter reflects the purpose of the field as a type descriptor. This improves code clarity.
118-121: Ohayo sensei! The change in entity type representation is a good move.Using a structured
Structinstance to represent the entity's type improves the code's ability to handle complex types in the future. This is a positive shift towards a more robust type system.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- crates/torii/core/src/query_queue.rs (3 hunks)
- crates/torii/grpc/src/server/mod.rs (1 hunks)
- crates/torii/grpc/src/server/subscriptions/entity.rs (3 hunks)
- crates/torii/grpc/src/server/subscriptions/event_message.rs (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/torii/core/src/query_queue.rs
Additional comments not posted (6)
crates/torii/grpc/src/server/subscriptions/event_message.rs (1)
79-80: Ohayo, sensei! The changes to the constructor look good.The removal of the
poolandmodel_cacheparameters simplifies the initialization of theServicestruct, aligning with the overall refactoring goal of reducing database dependencies.crates/torii/grpc/src/server/subscriptions/entity.rs (3)
85-86: Ohayo, sensei! The changes to thenewfunction look good.The removal of the
poolandmodel_cacheparameters simplifies the construction of theServiceand reduces its dependencies. This change aligns with the overall goal of refactoring the subscription handling to eliminate the need for database fetching.
Line range hint
89-209: Ohayo, sensei! The refactoring of thepublish_updatesfunction looks great.The removal of the
cacheandpoolparameters simplifies the function and reduces its dependencies on external resources. Using theupdated_modeldirectly from theEntityto construct the response eliminates the need for querying the database, potentially improving performance.The overall logic of the function remains intact, ensuring that subscribers receive updates based on their clauses.
187-192: Ohayo, sensei! Consider usingif letto safely handle theOptionvalues.The past review comment is still valid and applicable to this code segment. Using
unwrap()can cause the application to panic ifentity.updated_modelisNoneor ifas_struct()returnsNone.To enhance robustness, refactor the code to safely handle the
Optionvalues usingif let:-let model = entity.updated_model.as_ref().unwrap().as_struct().unwrap().clone(); +let model = if let Some(updated_model) = entity.updated_model.as_ref() { + if let Some(struct_model) = updated_model.as_struct() { + struct_model.clone() + } else { + // Handle the case where as_struct() returns None + // e.g., log an error or skip processing + return; // or appropriate error handling + } +} else { + // Handle the case where updated_model is None + // e.g., log an error or skip processing + return; // or appropriate error handling +};crates/torii/grpc/src/server/mod.rs (2)
109-113: Ohayo sensei! The service instantiation looks much cleaner now. LGTM!Simplifying the service constructors by reducing the number of dependencies enhances maintainability and reduces complexity. The changes align with the goal of streamlining the initialization process.
Line range hint
32-38: Skipping review ofbuild_composite_clauseas it is not part of the changed code segments.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2455 +/- ##
==========================================
+ Coverage 68.29% 68.36% +0.07%
==========================================
Files 365 365
Lines 48034 47951 -83
==========================================
- Hits 32805 32782 -23
+ Misses 15229 15169 -60 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
Bug Fixes
Refactor