refactor(torii-graphql): move away from external url #2753
Conversation
WalkthroughOhayo! The changes in this pull request streamline the Torii application by removing the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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: 0
🧹 Outside diff range and nitpick comments (2)
crates/torii/graphql/src/server.rs (1)
Line range hint
45-58: Ohayo sensei! Consider documenting the GraphQL subscription endpointNow that we've hardcoded the subscription endpoint to "graphql/ws", it would be helpful to document this in the function's documentation to make it clear for future developers.
Add this documentation above the playground_filter:
/// Returns a filter that serves the GraphQL playground. /// The subscription endpoint is configured to use "graphql/ws".bin/torii/src/main.rs (1)
Line range hint
286-292: Ohayo sensei! Consider improving the broker reconnection logicThe current implementation has two potential improvements:
- The hardcoded sleep duration could be configurable
- The error handling for broker.next() could be more robust
Consider this improvement:
- tokio::time::sleep(Duration::from_secs(1)).await; + const RECONNECTION_DELAY: Duration = Duration::from_secs(1); + tokio::time::sleep(RECONNECTION_DELAY).await; + tracing::debug!("Reconnecting to broker after model update");
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
bin/torii/src/main.rs(2 hunks)crates/torii/cli/src/args.rs(0 hunks)crates/torii/graphql/src/server.rs(2 hunks)
💤 Files with no reviewable changes (1)
- crates/torii/cli/src/args.rs
🔇 Additional comments (2)
crates/torii/graphql/src/server.rs (1)
51-54: Ohayo! Fix typo in comment and verify URL patching
The comment contains a typo ("hsoted" should be "hosted"). Also, let's verify the URL patching behavior.
bin/torii/src/main.rs (1)
281-281: LGTM! The server.new() call is correctly updated
The removal of external_url parameter is consistent with the changes in the GraphQL server implementation.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2753 +/- ##
=======================================
Coverage 56.02% 56.03%
=======================================
Files 427 427
Lines 54589 54572 -17
=======================================
- Hits 30586 30579 -7
+ Misses 24003 23993 -10 ☔ View full report in Codecov by Sentry. |
* refactor(torii-graphql): move away from external url * fmt * better patch * comment typo
Summary by CodeRabbit
New Features
external_urloption from CLI arguments and server setup.Bug Fixes
world_addressspecification.Refactor