-
Notifications
You must be signed in to change notification settings - Fork 771
Reducing print messages, moving msg to verbose #546
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
ac3fdf7
f3a407e
9c4561f
47bd944
70e884f
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 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,4 +1,4 @@ | ||||||||||||
"""Deploy command for MCP Agent Cloud CLI. | ||||||||||||
"""Deploy command for mcp-agent cloud CLI. | ||||||||||||
|
||||||||||||
This module provides the deploy_config function which processes configuration files | ||||||||||||
with secret tags and transforms them into deployment-ready configurations with secret handles. | ||||||||||||
|
@@ -9,7 +9,6 @@ | |||||||||||
from typing import Optional | ||||||||||||
|
||||||||||||
import typer | ||||||||||||
from rich.panel import Panel | ||||||||||||
from rich.progress import Progress, SpinnerColumn, TextColumn | ||||||||||||
|
||||||||||||
from mcp_agent.cli.auth import load_api_key_credentials | ||||||||||||
|
@@ -30,7 +29,6 @@ | |||||||||||
) | ||||||||||||
from mcp_agent.cli.utils.retry import retry_async_with_exponential_backoff, RetryError | ||||||||||||
from mcp_agent.cli.utils.ux import ( | ||||||||||||
console, | ||||||||||||
print_deployment_header, | ||||||||||||
print_error, | ||||||||||||
print_info, | ||||||||||||
|
@@ -130,7 +128,7 @@ def deploy_config( | |||||||||||
resolve_path=True, | ||||||||||||
), | ||||||||||||
) -> str: | ||||||||||||
"""Deploy an MCP agent using the specified configuration. | ||||||||||||
"""Deploy an mcp-agent using the specified configuration. | ||||||||||||
|
||||||||||||
An MCP App is deployed from bundling the code at the specified config directory. | ||||||||||||
This directory must contain an 'mcp_agent.config.yaml' at its root. The process will look for an existing | ||||||||||||
|
@@ -176,18 +174,15 @@ def deploy_config( | |||||||||||
if app_name is None: | ||||||||||||
if default_app_name: | ||||||||||||
print_info( | ||||||||||||
f"No app name provided. Using '{default_app_name}' from configuration." | ||||||||||||
f"Using app name from config.yaml: '{default_app_name}'" | ||||||||||||
) | ||||||||||||
app_name = default_app_name | ||||||||||||
else: | ||||||||||||
app_name = "default" | ||||||||||||
print_info("No app name provided. Using 'default' as app name.") | ||||||||||||
print_info("Using app name: 'default'") | ||||||||||||
|
||||||||||||
if app_description is None: | ||||||||||||
if default_app_description: | ||||||||||||
print_info( | ||||||||||||
"No app description provided. Using description from configuration." | ||||||||||||
) | ||||||||||||
app_description = default_app_description | ||||||||||||
|
||||||||||||
provided_key = api_key | ||||||||||||
|
@@ -203,40 +198,50 @@ def deploy_config( | |||||||||||
) | ||||||||||||
if not effective_api_key: | ||||||||||||
raise CLIError( | ||||||||||||
"Must be logged in to deploy. Run 'mcp-agent login', set MCP_API_KEY environment variable or specify --api-key option.", | ||||||||||||
"You need to be logged in to deploy.\n\n" | ||||||||||||
"To continue, do one of the following:\n" | ||||||||||||
" • Run: mcp-agent login\n" | ||||||||||||
" • Or set the MCP_API_KEY environment variable\n" | ||||||||||||
" • Or use the --api-key flag with your key", | ||||||||||||
retriable=False, | ||||||||||||
) | ||||||||||||
print_info(f"Using API at {effective_api_url}") | ||||||||||||
|
||||||||||||
if settings.VERBOSE: | ||||||||||||
print_info(f"Using API at {effective_api_url}") | ||||||||||||
|
||||||||||||
mcp_app_client = MCPAppClient( | ||||||||||||
api_url=effective_api_url, api_key=effective_api_key | ||||||||||||
) | ||||||||||||
|
||||||||||||
print_info(f"Checking for existing app ID for '{app_name}'...") | ||||||||||||
if settings.VERBOSE: | ||||||||||||
print_info(f"Checking for existing app ID for '{app_name}'...") | ||||||||||||
|
||||||||||||
try: | ||||||||||||
app_id = run_async(mcp_app_client.get_app_id_by_name(app_name)) | ||||||||||||
if not app_id: | ||||||||||||
print_info( | ||||||||||||
f"No existing app found with name '{app_name}'. Creating a new app..." | ||||||||||||
) | ||||||||||||
print_info(f"App '{app_name}' not found — creating a new one...") | ||||||||||||
app = run_async( | ||||||||||||
mcp_app_client.create_app( | ||||||||||||
name=app_name, description=app_description | ||||||||||||
) | ||||||||||||
) | ||||||||||||
app_id = app.appId | ||||||||||||
print_success(f"Created new app with ID: {app_id}") | ||||||||||||
print_success(f"Created new app '{app_name}'") | ||||||||||||
if settings.VERBOSE: | ||||||||||||
print_info(f"New app id: `{app_id}`") | ||||||||||||
else: | ||||||||||||
short_id = f"{app_id[:8]}…" | ||||||||||||
print_success( | ||||||||||||
f"Found existing app with ID: {app_id} for name '{app_name}'" | ||||||||||||
f"Found existing app '{app_name}' (ID: `{short_id}`)" | ||||||||||||
) | ||||||||||||
if not non_interactive: | ||||||||||||
use_existing = typer.confirm( | ||||||||||||
f"Do you want deploy an update to the existing app ID: {app_id}?", | ||||||||||||
f"Deploy an update to '{app_name}' (ID: `{short_id}`)?", | ||||||||||||
default=True, | ||||||||||||
) | ||||||||||||
if use_existing: | ||||||||||||
print_info(f"Will deploy an update to app ID: {app_id}") | ||||||||||||
if settings.VERBOSE: | ||||||||||||
print_info(f"Will deploy an update to app ID: `{app_id}`") | ||||||||||||
else: | ||||||||||||
print_error( | ||||||||||||
"Cancelling deployment. Please choose a different app name." | ||||||||||||
|
@@ -257,25 +262,21 @@ def deploy_config( | |||||||||||
# If a deployed secrets file already exists, determine if it should be used or overwritten | ||||||||||||
if deployed_secrets_file: | ||||||||||||
if secrets_file: | ||||||||||||
print_info( | ||||||||||||
f"Both '{MCP_SECRETS_FILENAME}' and '{MCP_DEPLOYED_SECRETS_FILENAME}' found in {config_dir}." | ||||||||||||
) | ||||||||||||
if settings.VERBOSE: | ||||||||||||
print_info( | ||||||||||||
f"Both '{MCP_SECRETS_FILENAME}' and '{MCP_DEPLOYED_SECRETS_FILENAME}' found in {config_dir}." | ||||||||||||
) | ||||||||||||
if non_interactive: | ||||||||||||
print_info( | ||||||||||||
"--non-interactive specified, using existing deployed secrets file without changes." | ||||||||||||
"Running in non-interactive mode — reusing previously deployed secrets." | ||||||||||||
) | ||||||||||||
else: | ||||||||||||
reuse = typer.confirm( | ||||||||||||
f"Do you want to reuse the previously deployed secrets in '{MCP_DEPLOYED_SECRETS_FILENAME}'?", | ||||||||||||
f"Re-use the deployed secrets from '{MCP_DEPLOYED_SECRETS_FILENAME}'?", | ||||||||||||
default=True, | ||||||||||||
) | ||||||||||||
if not reuse: | ||||||||||||
print_info( | ||||||||||||
f"Will update existing '{MCP_DEPLOYED_SECRETS_FILENAME}' by re-processing '{MCP_SECRETS_FILENAME}'." | ||||||||||||
) | ||||||||||||
deployed_secrets_file = None # Will trigger re-processing | ||||||||||||
else: | ||||||||||||
print_info(f"Using existing '{MCP_DEPLOYED_SECRETS_FILENAME}'.") | ||||||||||||
deployed_secrets_file = None # Will trigger re-processing) | ||||||||||||
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. There's a syntax error in this line - an extra closing parenthesis after the comment. Additionally, the original code included an Consider:
else:
if settings.VERBOSE:
print_info(f"Using existing '{MCP_DEPLOYED_SECRETS_FILENAME}'.") This maintains the user feedback while following the PR's goal of moving non-critical messages to verbose mode.
Suggested change
Spotted by Graphite Agent |
||||||||||||
else: | ||||||||||||
print_info( | ||||||||||||
f"Found '{MCP_DEPLOYED_SECRETS_FILENAME}' in {config_dir}, but no '{MCP_SECRETS_FILENAME}' to re-process. Using existing deployed secrets file." | ||||||||||||
|
@@ -287,7 +288,7 @@ def deploy_config( | |||||||||||
|
||||||||||||
secrets_transformed_path = None | ||||||||||||
if secrets_file and not deployed_secrets_file: | ||||||||||||
print_info("Processing secrets file...") | ||||||||||||
# print_info("Processing secrets file...") | ||||||||||||
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.
Suggested change
|
||||||||||||
secrets_transformed_path = config_dir / MCP_DEPLOYED_SECRETS_FILENAME | ||||||||||||
|
||||||||||||
run_async( | ||||||||||||
|
@@ -301,21 +302,14 @@ def deploy_config( | |||||||||||
) | ||||||||||||
|
||||||||||||
print_success("Secrets file processed successfully") | ||||||||||||
print_info( | ||||||||||||
f"Transformed secrets file written to {secrets_transformed_path}" | ||||||||||||
) | ||||||||||||
if settings.VERBOSE: | ||||||||||||
print_info( | ||||||||||||
f"Transformed secrets file written to {secrets_transformed_path}" | ||||||||||||
) | ||||||||||||
|
||||||||||||
else: | ||||||||||||
print_info("Skipping secrets processing...") | ||||||||||||
|
||||||||||||
console.print( | ||||||||||||
Panel( | ||||||||||||
"Ready to deploy MCP Agent with processed configuration", | ||||||||||||
title="Deployment Ready", | ||||||||||||
border_style="green", | ||||||||||||
) | ||||||||||||
) | ||||||||||||
|
||||||||||||
# Optionally create a local git tag as a breadcrumb of this deployment | ||||||||||||
if git_tag: | ||||||||||||
git_meta = get_git_metadata(config_dir) | ||||||||||||
|
@@ -325,7 +319,7 @@ def deploy_config( | |||||||||||
ts = datetime.now(timezone.utc).strftime("%Y%m%d-%H%M%S") | ||||||||||||
tag_name = f"mcp-deploy/{safe_name}/{ts}-{git_meta.short_sha}" | ||||||||||||
msg = ( | ||||||||||||
f"MCP Agent deploy for app '{app_name}' (id {app_id})\n" | ||||||||||||
f"mcp-agent deploy for app '{app_name}' (ID: `{app_id}`)\n" | ||||||||||||
f"Commit: {git_meta.commit_sha}\n" | ||||||||||||
f"Branch: {git_meta.branch or ''}\n" | ||||||||||||
f"Dirty: {git_meta.dirty}" | ||||||||||||
|
@@ -358,7 +352,7 @@ def deploy_config( | |||||||||||
) | ||||||||||||
) | ||||||||||||
|
||||||||||||
print_info(f"App ID: {app_id}") | ||||||||||||
print_info(f"App Name '{app_name}'") | ||||||||||||
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. Can we keep this as ID? Otherwise we lose that critical info for debugging |
||||||||||||
if app.appServerInfo: | ||||||||||||
status = ( | ||||||||||||
"ONLINE" | ||||||||||||
|
@@ -463,7 +457,8 @@ async def _perform_api_deployment(): | |||||||||||
raise | ||||||||||||
|
||||||||||||
if retry_count > 1: | ||||||||||||
print_info(f"Deployment API configured with up to {retry_count} attempts") | ||||||||||||
if settings.VERBOSE: | ||||||||||||
print_info(f"Deployment API configured with up to {retry_count} attempts") | ||||||||||||
|
||||||||||||
try: | ||||||||||||
return await retry_async_with_exponential_backoff( | ||||||||||||
|
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.
recommend including some substring from the end since the first 8 digits can definitely match other ids