Add documentation for prefect sdk generate CLI#1
Conversation
- Add auto-generated CLI reference for `prefect sdk` command - Add how-to guide for generating typed SDKs from deployments - Update navigation in docs.json for both new pages - Regenerate CLI docs to include new --output options Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Keep only sdk.mdx from the CLI docs regeneration. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The code examples reference a hypothetical my_sdk module that doesn't exist, so they can't be executed as tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove marketing-style "before/after" section - Use sentence case headers - Task-oriented organization (CLI then Python) - Add opening context and further reading links - Direct, instructional tone throughout Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove marketing-style "before/after" section - Use sentence case headers - Task-oriented organization (CLI then Python) - Add opening context and further reading links - Direct, instructional tone throughout Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move generate-custom-sdk.mdx to docs/v3/advanced/ - Create new "Deployments" group in Advanced section - Move form-building guide to new Deployments group - Both guides relate to deployment configuration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds documentation for the prefect sdk generate CLI command, which creates a typed Python SDK from workspace deployments. The PR includes comprehensive user-facing documentation and CLI reference, along with some modifications to the SDK fetcher implementation.
Changes:
- Added CLI reference documentation for the
prefect sdk generatecommand - Added comprehensive how-to guide for generating and using custom SDKs
- Reorganized documentation structure to group deployment-related advanced topics
- Modified fetcher.py with several code changes including logging additions and logic simplifications
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/v3/api-ref/cli/sdk.mdx | New CLI reference page documenting the prefect sdk generate command and its options |
| docs/v3/advanced/generate-custom-sdk.mdx | Comprehensive guide on generating custom SDKs, including usage examples and best practices |
| docs/docs.json | Updated navigation to include new documentation pages and reorganized deployment-related topics into a dedicated group |
| src/prefect/_sdk/fetcher.py | Added logging support and modified deployment filtering logic, work pool variable handling, and parallel fetching behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Logger for SDK fetcher operations | ||
| logger = logging.getLogger(__name__) |
There was a problem hiding this comment.
The logger is initialized before the imports from prefect._sdk.models. This violates Python import conventions where all imports should be grouped together at the top of the file. The logger initialization should be placed after all imports.
| variables = base_job_template["variables"] | ||
| if isinstance(variables, dict): | ||
| job_vars_schema = variables | ||
| job_vars_schema = base_job_template["variables"] |
There was a problem hiding this comment.
The removal of the type check for variables before assigning to job_vars_schema could cause issues if base_job_template["variables"] is not a dict. The original code defensively checked isinstance(variables, dict) before assignment. If the variables field can be a non-dict type (e.g., None, list, or other types), this change could introduce a bug where job_vars_schema contains unexpected data types, potentially causing issues downstream where a dict is expected.
| job_vars_schema = base_job_template["variables"] | |
| variables = base_job_template.get("variables") | |
| if isinstance(variables, dict): | |
| job_vars_schema = variables |
|
|
||
| work_pools: dict[str, WorkPoolInfo] = {} | ||
| for name, result in zip(pool_names_list, results, strict=True): | ||
| for name, result in zip(pool_names_list, results): |
There was a problem hiding this comment.
The removal of strict=True from the zip() call makes the code less safe. The strict=True parameter ensures that pool_names_list and results have the same length, which they should since results comes from gathering tasks created from pool_names_list. Removing this parameter means that if the lists somehow have mismatched lengths (due to a bug), the code will silently skip items instead of raising an error, making debugging harder.
| for name, result in zip(pool_names_list, results): | |
| for name, result in zip(pool_names_list, results, strict=True): |
| # If filtering by deployment name, check the full name matches | ||
| full_name = f"{flow_name}/{dep.name}" | ||
| if deployment_names and full_name not in deployment_names: | ||
| if deployment_names and dep.name not in deployment_names: |
There was a problem hiding this comment.
This change appears to be a bug. The logic on line 397 was checking if full_name (e.g., "my-flow/production") is not in deployment_names, but now it checks if dep.name (just "production") is not in deployment_names. This breaks the filtering logic when users provide full deployment names.
Looking at the code below (lines 402-413), there's explicit handling for both short names (without "/") and full names (with "/"). The original check was correct: when deployment_names contains entries, we need to check if either the full name matches OR if a short name matches. By changing this to check only dep.name, the code will now incorrectly skip deployments when users specify full names like "my-flow/production" in the filter, because dep.name would be just "production" which wouldn't be in the list.
| if deployment_names and dep.name not in deployment_names: | |
| if deployment_names and full_name not in deployment_names: |
Benchmark PR from agentic-review-benchmarks#1