mirror of
https://github.com/PrefectHQ/fastmcp.git
synced 2026-08-20 12:34:17 +02:00
fix: replace or with is not None checks for config/override merging (#3833)
* fix: replace `or` with `is not None` checks for config/override merging Falsy-but-valid values like port=0 (OS-assigned), host="" (all interfaces), and description="" (explicitly cleared) were silently dropped by `x or default` patterns across CLI, transport, and component registration. Fixes #3832 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: replace `or` with `is not None` for description in FunctionResourceTemplate 🤖 Generated with Claude Code Co-authored-by: Jeremiah Lowin <jlowin@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Co-authored-by: Jeremiah Lowin <jlowin@users.noreply.github.com>
This commit is contained in:
parent
4b59e0d94b
commit
790f0bcb47
8 changed files with 62 additions and 40 deletions
|
|
@ -521,13 +521,13 @@ async def run(
|
|||
|
||||
# Warn about options that are ignored in module mode
|
||||
ignored_options: list[str] = []
|
||||
if transport:
|
||||
if transport is not None:
|
||||
ignored_options.append("--transport")
|
||||
if host:
|
||||
if host is not None:
|
||||
ignored_options.append("--host")
|
||||
if port:
|
||||
if port is not None:
|
||||
ignored_options.append("--port")
|
||||
if path:
|
||||
if path is not None:
|
||||
ignored_options.append("--path")
|
||||
if ignored_options:
|
||||
logger.warning(
|
||||
|
|
@ -601,11 +601,15 @@ async def run(
|
|||
sys.exit(1)
|
||||
|
||||
# Get effective values (CLI overrides take precedence)
|
||||
final_transport = transport or config.deployment.transport
|
||||
final_host = host or config.deployment.host
|
||||
final_port = port or config.deployment.port
|
||||
final_path = path or config.deployment.path
|
||||
final_log_level = log_level or config.deployment.log_level
|
||||
final_transport = (
|
||||
transport if transport is not None else config.deployment.transport
|
||||
)
|
||||
final_host = host if host is not None else config.deployment.host
|
||||
final_port = port if port is not None else config.deployment.port
|
||||
final_path = path if path is not None else config.deployment.path
|
||||
final_log_level = (
|
||||
log_level if log_level is not None else config.deployment.log_level
|
||||
)
|
||||
final_server_args = server_args or config.deployment.args
|
||||
# Use CLI override if provided, otherwise use settings
|
||||
# no_banner CLI flag overrides the show_server_banner setting
|
||||
|
|
@ -642,11 +646,11 @@ async def run(
|
|||
if final_transport:
|
||||
reload_cmd.extend(["--transport", final_transport])
|
||||
if final_transport != "stdio":
|
||||
if final_host:
|
||||
if final_host is not None:
|
||||
reload_cmd.extend(["--host", final_host])
|
||||
if final_port:
|
||||
if final_port is not None:
|
||||
reload_cmd.extend(["--port", str(final_port)])
|
||||
if final_path:
|
||||
if final_path is not None:
|
||||
reload_cmd.extend(["--path", final_path])
|
||||
if final_log_level:
|
||||
reload_cmd.extend(["--log-level", final_log_level])
|
||||
|
|
@ -691,11 +695,11 @@ async def run(
|
|||
inner_cmd.extend(["--transport", final_transport])
|
||||
# Only add HTTP-specific options for non-stdio transports
|
||||
if final_transport != "stdio":
|
||||
if final_host:
|
||||
if final_host is not None:
|
||||
inner_cmd.extend(["--host", final_host])
|
||||
if final_port:
|
||||
if final_port is not None:
|
||||
inner_cmd.extend(["--port", str(final_port)])
|
||||
if final_path:
|
||||
if final_path is not None:
|
||||
inner_cmd.extend(["--path", final_path])
|
||||
if final_log_level:
|
||||
inner_cmd.extend(["--log-level", final_log_level])
|
||||
|
|
|
|||
|
|
@ -181,11 +181,15 @@ async def run_command(
|
|||
config = load_mcp_server_config(config_path)
|
||||
|
||||
# Merge deployment config with CLI arguments (CLI takes precedence)
|
||||
transport = transport or config.deployment.transport
|
||||
host = host or config.deployment.host
|
||||
port = port or config.deployment.port
|
||||
path = path or config.deployment.path
|
||||
log_level = log_level or config.deployment.log_level
|
||||
transport = (
|
||||
transport if transport is not None else config.deployment.transport
|
||||
)
|
||||
host = host if host is not None else config.deployment.host
|
||||
port = port if port is not None else config.deployment.port
|
||||
path = path if path is not None else config.deployment.path
|
||||
log_level = (
|
||||
log_level if log_level is not None else config.deployment.log_level
|
||||
)
|
||||
server_args = (
|
||||
server_args if server_args is not None else config.deployment.args
|
||||
)
|
||||
|
|
@ -234,15 +238,17 @@ async def run_command(
|
|||
return
|
||||
|
||||
kwargs = {}
|
||||
if transport:
|
||||
if transport is not None:
|
||||
kwargs["transport"] = transport
|
||||
if host:
|
||||
kwargs["host"] = host
|
||||
if port:
|
||||
kwargs["port"] = port
|
||||
if path:
|
||||
kwargs["path"] = path
|
||||
if log_level:
|
||||
# Only pass HTTP-specific options for non-stdio transports
|
||||
if transport != "stdio":
|
||||
if host is not None:
|
||||
kwargs["host"] = host
|
||||
if port is not None:
|
||||
kwargs["port"] = port
|
||||
if path is not None:
|
||||
kwargs["path"] = path
|
||||
if log_level is not None:
|
||||
kwargs["log_level"] = log_level
|
||||
if stateless:
|
||||
kwargs["stateless"] = True
|
||||
|
|
@ -310,9 +316,9 @@ async def run_v1_server_async(
|
|||
port: Port to bind to
|
||||
transport: Transport protocol to use
|
||||
"""
|
||||
if host:
|
||||
if host is not None:
|
||||
server.settings.host = host
|
||||
if port:
|
||||
if port is not None:
|
||||
server.settings.port = port
|
||||
|
||||
match transport:
|
||||
|
|
|
|||
|
|
@ -152,7 +152,11 @@ class FunctionPrompt(Prompt):
|
|||
if param.kind == inspect.Parameter.VAR_KEYWORD:
|
||||
raise ValueError("Functions with **kwargs are not supported as prompts")
|
||||
|
||||
description = metadata.description or inspect.getdoc(fn)
|
||||
description = (
|
||||
metadata.description
|
||||
if metadata.description is not None
|
||||
else inspect.getdoc(fn)
|
||||
)
|
||||
|
||||
# Normalize task to TaskConfig and validate
|
||||
task_value = metadata.task
|
||||
|
|
|
|||
|
|
@ -196,7 +196,9 @@ class FunctionResource(Resource):
|
|||
name=func_name,
|
||||
version=str(metadata.version) if metadata.version is not None else None,
|
||||
title=metadata.title,
|
||||
description=metadata.description or inspect.getdoc(fn),
|
||||
description=metadata.description
|
||||
if metadata.description is not None
|
||||
else inspect.getdoc(fn),
|
||||
icons=metadata.icons,
|
||||
mime_type=resolved_mime or "text/plain",
|
||||
tags=metadata.tags or set(),
|
||||
|
|
|
|||
|
|
@ -553,7 +553,7 @@ class FunctionResourceTemplate(ResourceTemplate):
|
|||
f"URI parameters {all_uri_params} must be a subset of the function arguments: {func_params}"
|
||||
)
|
||||
|
||||
description = description or inspect.getdoc(fn)
|
||||
description = description if description is not None else inspect.getdoc(fn)
|
||||
|
||||
# Normalize task to TaskConfig and validate
|
||||
if task is None:
|
||||
|
|
|
|||
|
|
@ -263,9 +263,11 @@ class TransportMixin:
|
|||
if stateless_http and transport == "sse":
|
||||
raise ValueError("SSE transport does not support stateless mode")
|
||||
|
||||
host = host or fastmcp.settings.host
|
||||
port = port or fastmcp.settings.port
|
||||
default_log_level_to_use = (log_level or fastmcp.settings.log_level).lower()
|
||||
host = host if host is not None else fastmcp.settings.host
|
||||
port = port if port is not None else fastmcp.settings.port
|
||||
default_log_level_to_use = (
|
||||
log_level if log_level is not None else fastmcp.settings.log_level
|
||||
).lower()
|
||||
|
||||
app = self.http_app(
|
||||
path=path,
|
||||
|
|
@ -335,7 +337,9 @@ class TransportMixin:
|
|||
if transport in ("streamable-http", "http"):
|
||||
return create_streamable_http_app(
|
||||
server=self,
|
||||
streamable_http_path=path or fastmcp.settings.streamable_http_path,
|
||||
streamable_http_path=path
|
||||
if path is not None
|
||||
else fastmcp.settings.streamable_http_path,
|
||||
event_store=event_store,
|
||||
retry_interval=retry_interval,
|
||||
auth=self.auth,
|
||||
|
|
@ -356,7 +360,7 @@ class TransportMixin:
|
|||
return create_sse_app(
|
||||
server=self,
|
||||
message_path=fastmcp.settings.message_path,
|
||||
sse_path=path or fastmcp.settings.sse_path,
|
||||
sse_path=path if path is not None else fastmcp.settings.sse_path,
|
||||
auth=self.auth,
|
||||
debug=fastmcp.settings.debug,
|
||||
middleware=middleware,
|
||||
|
|
|
|||
|
|
@ -116,7 +116,7 @@ class SamplingTool(FastMCPBaseModel):
|
|||
|
||||
return cls(
|
||||
name=name or parsed.name,
|
||||
description=description or parsed.description,
|
||||
description=description if description is not None else parsed.description,
|
||||
parameters=parsed.input_schema,
|
||||
fn=parsed.fn,
|
||||
sequential=sequential,
|
||||
|
|
|
|||
|
|
@ -222,7 +222,9 @@ class FunctionTool(Tool):
|
|||
name=metadata.name or parsed_fn.name,
|
||||
version=str(metadata.version) if metadata.version is not None else None,
|
||||
title=metadata.title,
|
||||
description=metadata.description or parsed_fn.description,
|
||||
description=metadata.description
|
||||
if metadata.description is not None
|
||||
else parsed_fn.description,
|
||||
icons=metadata.icons,
|
||||
parameters=parsed_fn.input_schema,
|
||||
output_schema=final_output_schema,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue