Skip to content

[Router] Consolidate ConnectionMode enum to core module#11937

Merged
slin1237 merged 1 commit intosgl-project:mainfrom
YouNeedCryDear:merge-connection-mode
Oct 23, 2025
Merged

[Router] Consolidate ConnectionMode enum to core module#11937
slin1237 merged 1 commit intosgl-project:mainfrom
YouNeedCryDear:merge-connection-mode

Conversation

@YouNeedCryDear
Copy link
Contributor

@YouNeedCryDear YouNeedCryDear commented Oct 21, 2025

Summary

Consolidates the ConnectionMode enum definition from the configuration layer (config/types.rs) to the core domain layer (core/worker.rs), eliminating code duplication and improving type safety.

Motivation

Addresses #11901

Previously, ConnectionMode existed in two separate locations:

  • config/types.rs - Simple enum for configuration serialization
  • core/worker.rs - Enum with additional fields for worker communication

This duplication required conversion logic and created a maintenance burden. The consolidation establishes a single source of truth and improves the domain model.

Changes

Core Changes

  • Moved ConnectionMode enum from config/types.rs to core/worker.rs
  • Enhanced enum with Serialize and Deserialize traits for configuration interop
  • Changed Grpc variant from unit to struct: Grpc { port: Option<u16> }
  • Removed redundant convert_connection_mode() function from worker_manager.rs

Updated Files

  • sgl-router/src/config/types.rs - Removed ConnectionMode definition
  • sgl-router/src/core/worker.rs - Added ConnectionMode with serde support
  • sgl-router/src/config/validation.rs - Updated imports and pattern matching
  • sgl-router/src/core/worker_manager.rs - Updated imports, removed conversion function
  • sgl-router/src/lib.rs - Updated imports and return types
  • sgl-router/src/main.rs - Updated imports
  • sgl-router/src/routers/factory.rs - Updated imports and pattern matching
  • sgl-router/src/routers/router_manager.rs - Updated imports and pattern matching
  • sgl-router/src/server.rs - Updated imports and pattern matching
  • Test files updated accordingly

Pattern Matching Updates

All pattern matches updated to handle struct variant:

// Before
ConnectionMode::Grpc => { ... }

// After  
ConnectionMode::Grpc { .. } => { ... }

Benefits

  1. Eliminates duplication - Single source of truth for connection mode
  2. Improved type safety - Port configuration embedded directly in enum variant
  3. Better domain model - ConnectionMode semantically belongs with Worker entities
  4. Reduced complexity - Removed error-prone conversion logic between config and core types
  5. Easier extensibility - Adding new connection modes or parameters is simpler

Backward Compatibility

  • Fully backward compatible for configuration files
  • Old format {"type": "grpc"} still deserializes correctly
  • New format {"type": "grpc", "port": 50051} supported via optional port field
  • No impact on Python CLI user interactions (connection mode auto-detected from URLs)
  • All existing tests pass with updated pattern matching

Testing

  • Updated all affected unit tests
  • Updated integration tests in tests/ directory
  • All pattern matching validated by Rust compiler
  • Verified backward compatibility with configuration parsing

Additional Notes

This change makes it easier to add an explicit --connection-mode CLI flag in the future if needed, as ConnectionMode now has proper serialization support throughout the codebase.

Integration Test Screenshots

4 Llama 3.1 8B with HTTP router and service discovery
Screenshot 2025-10-22 at 11 11 51 PM

1P2D Llama 4 Mav with gRPC router
Screenshot 2025-10-22 at 11 41 18 PM

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @YouNeedCryDear, 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 refactors the handling of connection modes within the router by consolidating the ConnectionMode enum into a single, authoritative location in the core module. This change not only eliminates code duplication and simplifies the codebase but also enhances type safety by integrating gRPC port configuration directly into the enum variant. The refactoring improves the overall domain model and prepares the system for easier future extensibility, all while ensuring full backward compatibility with existing configurations.

Highlights

  • Enum Consolidation: The ConnectionMode enum definition has been moved from config/types.rs to core/worker.rs, centralizing its definition and eliminating duplication.
  • Enhanced Type Safety and Serialization: The ConnectionMode enum in core/worker.rs now includes Serialize and Deserialize traits, and its Grpc variant has been enhanced to a struct Grpc { port: Option<u16> }, allowing the gRPC port to be directly embedded within the enum variant.
  • Removed Redundant Conversion Logic: The convert_connection_mode() function in worker_manager.rs has been removed, as the consolidated enum no longer requires explicit conversion between configuration and core types.
  • Pattern Matching Updates: All instances of pattern matching for ConnectionMode::Grpc have been updated to ConnectionMode::Grpc { .. } to correctly handle the new struct variant.
  • Backward Compatibility: The changes maintain full backward compatibility for configuration files, allowing both old ({"type": "grpc"}) and new ({"type": "grpc", "port": 50051}) formats to be deserialized correctly.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This is a great refactoring that consolidates the ConnectionMode enum, removing duplication and improving type safety by embedding the port configuration directly in the enum. The changes are well-executed across the codebase, with all necessary updates to imports and pattern matching. The use of serde attributes ensures backward compatibility for configuration files, which is a thoughtful touch.

I have one suggestion for a potential improvement to make the handling of ConnectionMode even more consistent.

@slin1237 slin1237 self-assigned this Oct 22, 2025
@YouNeedCryDear YouNeedCryDear force-pushed the merge-connection-mode branch 7 times, most recently from 5219380 to c23f810 Compare October 23, 2025 05:20
use config clone for grpc worker init
@slin1237 slin1237 merged commit 53c2934 into sgl-project:main Oct 23, 2025
37 of 38 checks passed
@YouNeedCryDear YouNeedCryDear deleted the merge-connection-mode branch October 23, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments