Conversation
kavmur
changed the title
|
Cool! My high level thoughts:
|
| ### Background Job Submission | ||
| The job submission process is implemented using a background thread pattern to handle long-running operations: |
There was a problem hiding this comment.
This doesn't really make sense to me. Is this the LLM thinking that Deadline is 'the background thread'
There was a problem hiding this comment.
This is mainly to avoid MCP timeout error for jobs with large attachments. We could get rid of this code and keep the submit tool synchronous if we increase the timeout values in MCP config settings, but that is a manual step.
There was a problem hiding this comment.
hm interesting. this is a client configuration?
There was a problem hiding this comment.
yes, this is a client configuration. If we want to keep the tool blocking we will need a larger timeout in client config json or patched in client request: https://github.com/modelcontextprotocol/typescript-sdk/blob/main/src/shared/protocol.ts#L593
There was a problem hiding this comment.
I switched this back to blocking to make the tool much simpler. We can add progress reporting once that is supported by more clients: https://modelcontextprotocol.io/specification/2025-03-26/basic/utilities/progress
|
|
||
| 1. Install the AWS Deadline Cloud client package: | ||
| bash | ||
| pip install deadline |
| ### Starting the MCP Server | ||
|
|
||
| ```sh | ||
| $ deadline-mcp |
There was a problem hiding this comment.
my preference is we make this like deadline mcp-server and not add more things to the path. we already have the CLI infrastructure to easily add commands.
| if not destination: | ||
| destination = root | ||
|
|
||
| logger = ClickLogger(is_json=True) |
There was a problem hiding this comment.
seems we should configure the logger at the entry point. at somepoint this MCP server could run in a service environment and using this ClickLogger won't make sense.
There was a problem hiding this comment.
Removed this tool as per feedback. We are only exposing public api functions
|
|
||
| if not result: | ||
| return json.dumps({ | ||
| "status": "NO_FILES", |
There was a problem hiding this comment.
is this status thing part of MCP or just a convention?
There was a problem hiding this comment.
Not part of MCP, just a convention for returning LLM friendly messages.
| from deadline.job_attachments.download import OutputDownloader | ||
| from deadline.job_attachments.models import FileConflictResolution, JobAttachmentS3Settings |
There was a problem hiding this comment.
we have a lot of imports inside of functions. why is that? we should avoid this if at all possible. the reasons I've seen people resort to it is usually a giant code smell(circular dependencies), but in this case I'm guessing your LLM was hyper focused on this function and didn't want to touch the beginning of the file or something? I've definitely seen that. A good ruff rule should fix this.
|
|
||
| deadline = api.get_boto3_client("deadline") | ||
| queue = deadline.get_queue(farmId=farm_id, queueId=queue_id) | ||
| queue_role_session = api.get_queue_user_boto3_session( |
There was a problem hiding this comment.
hm the MCP server should probably hang on to a shared credential provider so every request doesn't perform a new refresh
There was a problem hiding this comment.
I tried creating a cached queue user session with shared credential provider, but this lead to more duplicate code as I cannot reuse the CLI's helpers directly. And I think shared user sessions only make sense for mcp server (since it is long running/multi tenant) vs a CLI. Also we do use @lru_cache which provides some level of caching.
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
|
|
||
| """ | ||
| AWS Deadline Cloud MCP placeholder tools. |
There was a problem hiding this comment.
Personally I don't think we should have this file but let's discuss! if we really want we could add a single resource that describes all this. Adding a ton of tools that the LLM gets to call and waste an entire context window just to find out it doesn't do anything to everyones context is not frugal
There was a problem hiding this comment.
Makes sense to remove it to save context space. I was thinking this would help LLM search for the right tool, but we don't need to create a bunch of tools for that, one generic response/prompt for any api that is not available should work.
There was a problem hiding this comment.
Could we try to use mcp.prompt to combine all these tools into one ? e.g.
@app.prompt()
def farm_management() -> str:
"""Call the following farm management APIs
-CreateFarm
-UpdateFarm
-...
"""
return "This operation is not supported through the Deadline client API. Please use the AWS CLI, AWS Console, or the aws-documentation-mcp-server for creating farms."
There was a problem hiding this comment.
Thanks, @app.prompt is a good idea to combine everything, but I think will still occupy a lot of context space. Especially for info that agents can easily figure out from other docs/context. So will remove all placeholder/generic tools and focus only on ones that are exposed in deadline cli
| result = api.list_queues(farmId=farm_id) | ||
| queues = result.get('queues', []) |
| queue_id: The ID of the queue to retrieve | ||
| """ | ||
| try: | ||
| result = api.list_queues(farmId=farm_id) |
| ``` | ||
|
|
||
|
|
||
| Shared storage is possible with customer-managed fleets (CMF) but not service-managed fleets (SMF). See [shared storage][shared-storage] for more information. |
There was a problem hiding this comment.
This change doesn't look related to the MCP.
|
|
||
| ## Architecture Overview | ||
|
|
||
| The AWS Deadline Cloud MCP Server uses FastMCP within the MCP Python SDK to provide a simplified, decorator-based approach to tool registration while maintaining full MCP protocol compliance. |
There was a problem hiding this comment.
This doc doesn't define the MCP acronym, or what the purpose of an MCP is. That would be great to describe in this section, along with an overview of what you can accomplish when you use this MCP.
| - `submit_job(farm_id: str, queue_id: str, bundle_path: str, job_name: str = None, priority: int = 50)`: Submit new job from bundle (non-blocking with background processing) | ||
| - `check_job_submission(submission_id: str)`: Check status of background job submission | ||
| - `cancel_job(farm_id: str, queue_id: str, job_id: str, mark_as: str = "CANCELED")`: Cancel running job | ||
| - `download_job_output(farm_id: str, queue_id: str, job_id: str, destination_dir: str = None)`: Download job output files |
There was a problem hiding this comment.
I think this interface needs some more thought. What does destination_dir do when the job has inputs and outputs spread across three different drive letters?
| - `create_queue(farm_id: str, display_name: str, description: str = None)`: Create queue | ||
| - `update_queue(farm_id: str, queue_id: str, display_name: str = None, description: str = None)`: Update queue | ||
| - `delete_queue(farm_id: str, queue_id: str)`: Delete queue | ||
| - `get_job_logs(farm_id: str, queue_id: str, job_id: str)`: Get job logs |
| ### Manual Testing | ||
| ```bash | ||
| # Start the MCP server | ||
| deadline-mcp |
|
As an idea, what about putting your prompt into a PR and getting feedback there? It'd be easier to review and it'd be easier for you as the author to make changes. Then once the prompt/design feels right, you can launch an agent at it. |
|
Another thought: I think the public interface for the MCP server is the agent, not the MCP tools themselves. For backwards compatibility, we should be thoughtful about how an agent's behavior may change but I think we can iterate on the tools pretty freely. Doubly so if we throw an "experimental" flag on it. |
| ## Tool Organization | ||
|
|
||
| Tools are organized by functional area: |
There was a problem hiding this comment.
It'd be helpful to have guidance on what becomes a tool. As a first guess, maybe expose each CLI command as a tool?
53dd1e6 to
afec5ee
Compare
| **From `deadline.client.api._get_apis`:** | ||
| - `deadline_get_farm()`: Get detailed farm information | ||
|
|
||
| **From `deadline.client.api._session`:** |
There was a problem hiding this comment.
Most of these don't look like they'd made good MCP tools. Some return things that aren't serializable as text like boto3 clients.
There was a problem hiding this comment.
Added an allowlist (as suggested below) to the auto register function, so we expose only selected apis that would make useful tools.
| ## Usage Examples | ||
|
|
||
| ### Common Operations |
There was a problem hiding this comment.
Not sure this info is useful since MCP tools are not used directly by users and agents don't see this user guide.
| ### Testing the Server | ||
|
|
||
| **Start the MCP server:** | ||
| ```bash | ||
| deadline mcp-server | ||
| ``` | ||
|
|
||
| **Test with MCP client tools or IDE integrations that support MCP** |
There was a problem hiding this comment.
The MCP server should not be stared directly, right? To test it, it need to be added to an agent's config file.
There was a problem hiding this comment.
True, general usage would be adding this command to an agent's config or mcp client config. But for testing I found it to be useful if we are using a custom mcp client or mcp-inspector. I can reword it.
| ## Prerequisites | ||
| - Python 3.8 through 3.13 | ||
| - AWS Deadline Cloud client | ||
| - AWS credentials configured |
There was a problem hiding this comment.
Nit: remove this. Python requirements are documented elsewhere, and the Deadline Client and AWS creds are set up below.
| 4. **Set up default configuration (recommended):** | ||
| ```bash | ||
| deadline config set defaults.farm_id <your-farm-id> | ||
| deadline config set defaults.queue_id <your-queue-id> | ||
| deadline config set defaults.storage_profile_id <your-storage-profile-id> | ||
| ``` |
There was a problem hiding this comment.
Should the agent be able to do this?
There was a problem hiding this comment.
Good point, agents should ask for required fields based on tools signature if they are not provided in request and dont use config. I can remove them.
| inspect.Parameter('nextToken', inspect.Parameter.KEYWORD_ONLY, default=None, annotation=str), | ||
| inspect.Parameter('principalId', inspect.Parameter.KEYWORD_ONLY, default=None, annotation=str), | ||
| inspect.Parameter('maxResults', inspect.Parameter.KEYWORD_ONLY, default=None, annotation=str), | ||
| inspect.Parameter('studioId', inspect.Parameter.KEYWORD_ONLY, default=None, annotation=str), |
There was a problem hiding this comment.
There shouldn't be any studio references in the codebase right?
| return {k: v for k, v in kwargs.items() if v is not None and v != "" and v != "null"} | ||
|
|
||
|
|
||
| def _create_telemetry_wrapper(original_func, func_name: str, tool_serializer, tool_error_handler): |
There was a problem hiding this comment.
Why is this function so much smaller than the regular wrapper below? Seems like they should be similar? This function should just be ignoring the telemetry stuff right?
| return wrapper | ||
|
|
||
|
|
||
| def auto_register_api_tools( |
There was a problem hiding this comment.
Since not all API functions are useful for agents (e.g. boto3 client constructors), maybe use an allowlist. Like:
tools = [
api.list_farms,
api.get_farm,
...
]
A list lets us choose more intentionally what tools we expose while still using this automation you made so that the tools and APIs stay consistent.
There was a problem hiding this comment.
Yes, allowlist makes more sense. Fixed.
| from deadline.mcp.tools.job import submit_job | ||
|
|
||
|
|
||
| class TestMCPServer: |
There was a problem hiding this comment.
It'd be great to have a test which starts the real MCP server, then 1) uses an MCP client to connect to call ListTools and 2) actually invokes a tool and gets back a valid response.
| ```bash | ||
| # Replace with your AWS Account ID | ||
| export SERVICE_ACCOUNT_ID=000000000000 | ||
| export SERVICE_ACCOUNT_ID=170849269209 |
There was a problem hiding this comment.
Nit: we'll need to revert these before merging
|
|
||
| ### Advanced Workflows | ||
| ``` | ||
| "Help me troubleshoot why my render job failed" |
There was a problem hiding this comment.
Have we tested this workflow? Can this succeeded root cause the render job failure reason?
| from ...job_attachments.models import FileConflictResolution | ||
|
|
||
|
|
||
| # TODO: Make submit_job tool async once progress reporting feature is supported in clients |
There was a problem hiding this comment.
nit: You mentioned 1 GB upload takes less than 50 seconds, do we need this comment here too ?
| # Call the function with a mock bundle path and required parameters | ||
| result = submit_job("/mock/path", name="Test Job", priority=50, farm_id="farm-123", queue_id="queue-456") | ||
|
|
||
| # The function returns a dictionary with error information due to missing config |
There was a problem hiding this comment.
nit: Based on the test name, is this test case expected to succeeded to submit the job ? Otherwise, we may need to change the test name
|
|
||
|
|
||
| class TestMCPServer: | ||
| """Test the MCP server functionality.""" |
There was a problem hiding this comment.
Do we also need to have unit tests to test the tool being auto registered , otherwise we are not sure whether they have been auto registered successfully or not
985cae4 to
4eb5144
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| *, | ||
| purpose: JobBundlePurpose, | ||
| ) -> dict[str, Any]: ... | ||
| ) -> Optional[dict[str, Any]]: ... |
Check notice
Code scanning / CodeQL
Statement has no effect Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 12 months ago
To fix the problem, replace the ... statement at the end of the __call__ method in the OnCreateJobBundleCallback protocol with a pass statement. This makes it clear that the method is intentionally left unimplemented, as is standard for protocol or abstract method definitions in Python. The change should be made in the file src/deadline/client/ui/dialogs/submit_job_to_deadline_dialog.py, specifically on line 66. No additional imports or definitions are required.
| @@ -63,7 +63,8 @@ | ||
| host_requirements: Optional[Dict[str, Any]] = None, | ||
| *, | ||
| purpose: JobBundlePurpose, | ||
| ) -> Optional[dict[str, Any]]: ... | ||
| ) -> Optional[dict[str, Any]]: | ||
| pass | ||
|
|
||
|
|
||
| class SubmitJobToDeadlineDialog(QDialog): |
9b34bab to
560972d
Compare
cd09b3e to
f013217
Compare
02d47c9 to
fb8e817
Compare
Signed-off-by: kavmur <kavmur@users.noreply.github.com>
6 participants
What was the problem/requirement? (What/Why)
AWS Deadline Cloud users need a way to interact with their render farms through AI assistants and natural language interfaces. Currently, users must use CLI commands or Python APIs directly, which creates friction for AI-assisted workflows and automation. The Model Context Protocol (MCP) provides a standardized way for AI assistants to interact with external services, but AWS Deadline Cloud lacked MCP integration.
What was the solution? (How)
Add a MCP server to client tools to provide a simplified, decorator-based approach to tool registration while maintaining full MCP protocol compliance.
The server automatically registers all available Deadline Cloud API functions as MCP tools, plus additional specialized tools for job management.
What is the impact of this change?
How was this change tested?
Test Status:
downloadorasset_syncmodules? - No changes to these modulesWas this change documented?
Does this PR introduce new dependencies?
New Dependencies Added:
mcp >= 0.1.0- Model Context Protocol Python SDK (optional dependency for MCP functionality)pydantic >= 2.0.0- Data validation library used by MCP SDKJustification: These dependencies are added as optional dependencies under the
[mcp]extra, so they don't affect existing users. The MCP SDK is the standard library for MCP server implementation, and Pydantic is a widely-used validation library with minimal additional dependencies.Is this a breaking change?
No, this is not a breaking change. This PR is purely additive:
deadline-mcpcommandDoes this change impact security?
No significant security impact:
The MCP server runs locally and communicates via stdio, maintaining the same security posture as the existing CLI tools.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.