-
Notifications
You must be signed in to change notification settings - Fork 49
fix(python): add router arg fallback disable flag #1072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ea8ee2e
da9d8b9
96c5e15
6969e2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -241,6 +241,17 @@ def add_cli_args( | |
| "Control Plane Authentication", "API key and JWT/OIDC authentication" | ||
| ) | ||
|
|
||
| if use_router_prefix: | ||
| parser.add_argument( | ||
| "--router-disable-arg-fallback", | ||
| action="store_true", | ||
| default=False, | ||
| help=( | ||
| "When set, only use explicitly provided --router-* arguments and do not" | ||
| " fall back to backend arguments with the same name." | ||
| ), | ||
| ) | ||
|
|
||
| # Worker configuration | ||
| if not exclude_host_port: | ||
| worker_group.add_argument( | ||
|
|
@@ -1107,6 +1118,7 @@ def from_cli_args(cls, args: argparse.Namespace, use_router_prefix: bool = False | |
| prefix = "router_" if use_router_prefix else "" | ||
| cli_args_dict = vars(args) | ||
| args_dict = {} | ||
| disable_arg_fallback = bool(cli_args_dict.get(f"{prefix}disable_arg_fallback", False)) | ||
|
|
||
| for attr in dataclasses.fields(cls): | ||
| # Auto strip prefix from args. | ||
|
|
@@ -1117,7 +1129,11 @@ def from_cli_args(cls, args: argparse.Namespace, use_router_prefix: bool = False | |
| prefixed_key = f"{prefix}{attr.name}" | ||
| if prefixed_key in cli_args_dict and cli_args_dict[prefixed_key] is not None: | ||
| args_dict[attr.name] = cli_args_dict[prefixed_key] | ||
| elif attr.name in cli_args_dict and cli_args_dict[attr.name] not in (None, ""): | ||
| elif ( | ||
| not disable_arg_fallback | ||
| and attr.name in cli_args_dict | ||
| and cli_args_dict[attr.name] not in (None, "") | ||
| ): | ||
|
Comment on lines
+1132
to
+1136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With Useful? React with 👍 / 👎.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense |
||
| args_dict[attr.name] = cli_args_dict[attr.name] | ||
|
|
||
| # Special handling for CLI args with dashes vs dataclass fields with underscores | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabling fallback here also drops
host/portinheritance even thoughsmg serve/launch_serverregister router args withexclude_host_port=True(bindings/python/src/smg/serve.pylines 540/554 andbindings/python/src/smg/launch_server.pyline 161), so the router has no--router-host/--router-portsource. With--router-disable-arg-fallback,RouterArgs.from_cli_argsno longer reads unprefixed--host/--port, and the router silently reverts to dataclass defaults (0.0.0.0:30000) instead of the serve endpoint requested via CLI, which can bind the gateway to the wrong address/port.Useful? React with 👍 / 👎.