-
Notifications
You must be signed in to change notification settings - Fork 18
Add Invocation Commands #866
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
base: staging
Are you sure you want to change the base?
Conversation
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughIntroduces a DeploymentCodeCollapsible UI showing an “Endpoint” panel with tabs (CLI, Python, conditional cURL) that display command/code snippets. Adds command-builder utilities (buildCurl, buildZenCommand, buildPythonCommand) and corresponding tests. Integrates the new component into the deployments detail page’s right-hand container. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Deployment Detail Page
participant UI as DeploymentCodeCollapsible
participant Builder as command-builder.ts
participant Snippet as Codesnippet
User->>Page: Open deployment detail
Page->>UI: Render with deployment props
UI->>UI: Determine url, authKey, defaultBody
UI->>Builder: buildZenCommand(name, body)
Builder-->>UI: CLI command string
UI->>Builder: buildPythonCommand({deploymentId, defaultBody})
Builder-->>UI: Python snippet string
alt deployment.body.url exists
UI->>Builder: buildCurl(url, body, authKey)
Builder-->>UI: cURL command string
else
UI-->>UI: Hide cURL tab
end
UI->>Snippet: Render selected tab content
User->>UI: Toggle collapse / switch tabs
UI-->>User: Display corresponding snippet
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/deployments/[deploymentId]/_components/command-builder.ts (1)
1-24
: Consider simplifying the indentation logic.The curl command formatting works correctly, but line 13's indentation approach is somewhat convoluted:
const curlParams = JSON.stringify(defaultBody, null, 2).split("\n").join("\n ");This splits and rejoins to add indentation. While functional, it could be clearer.
Consider this alternative that's more explicit about the transformation:
-const curlParams = JSON.stringify(defaultBody, null, 2).split("\n").join("\n "); +const curlParams = JSON.stringify(defaultBody, null, 2) + .split("\n") + .map((line, idx) => (idx === 0 ? line : ` ${line}`)) + .join("\n");Or use a helper function:
function indentJson(obj: unknown, spaces: number = 4): string { const json = JSON.stringify(obj, null, 2); const indent = ' '.repeat(spaces); return json.split('\n').map((line, i) => i === 0 ? line : indent + line).join('\n'); }The current implementation is acceptable, but these alternatives make the intent clearer.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/deployments/[deploymentId]/_components/code-collapsible.tsx
(1 hunks)src/app/deployments/[deploymentId]/_components/command-builder.spec.ts
(1 hunks)src/app/deployments/[deploymentId]/_components/command-builder.ts
(1 hunks)src/app/deployments/[deploymentId]/page.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.tsx
⚙️ CodeRabbit configuration file
src/**/*.tsx
: Review React components for adherence to the ZenML design system, accessibility standards, and efficient state management.
Files:
src/app/deployments/[deploymentId]/page.tsx
src/app/deployments/[deploymentId]/_components/code-collapsible.tsx
src/**/*.ts
⚙️ CodeRabbit configuration file
src/**/*.ts
: Evaluate TypeScript modules and hooks for strict typing, predictable side effects, and consistent error handling.
Files:
src/app/deployments/[deploymentId]/_components/command-builder.spec.ts
src/app/deployments/[deploymentId]/_components/command-builder.ts
src/**/*.spec.ts*
⚙️ CodeRabbit configuration file
src/**/*.spec.ts*
: Ensure Vitest unit tests stay deterministic, include clear assertions, and reset mocks, timers, and TanStack Query caches.
Files:
src/app/deployments/[deploymentId]/_components/command-builder.spec.ts
🔇 Additional comments (7)
src/app/deployments/[deploymentId]/page.tsx (1)
1-1
: LGTM! Clean integration.The new
DeploymentCodeCollapsible
is properly imported and integrated into the layout with consistent spacing.Also applies to: 13-16
src/app/deployments/[deploymentId]/_components/code-collapsible.tsx (1)
26-34
: LGTM! Safe property access.The component correctly handles optional chaining for
deployment.body?.url
anddeployment.metadata?.auth_key
, with appropriate fallback toundefined
.src/app/deployments/[deploymentId]/_components/command-builder.spec.ts (4)
1-106
: Excellent test coverage for buildCurl.The test suite thoroughly covers happy paths (basic command, auth header, various body types) and edge cases (empty inputs, primitives, nested objects). The assertions verify both structure and content.
108-237
: Comprehensive buildZenCommand tests.Good coverage including mixed parameter types, null handling, fallback scenarios, and edge cases like empty objects and special characters.
178-183
: Test documents trailing space behavior.The test explicitly expects a trailing space when an empty object is passed:
"zenml deployment invoke my-deployment "
. This documents the current implementation behavior but suggests a minor quality issue in the command builder.Consider whether the implementation should be adjusted to avoid the trailing space for empty objects (see review comments on command-builder.ts).
239-378
: Thorough buildPythonCommand test coverage.The tests validate happy paths, edge cases, and fallback behavior consistently. The nested object parseability checks (lines 339-366) are particularly valuable.
src/app/deployments/[deploymentId]/_components/command-builder.ts (1)
43-68
: LGTM! Clean conditional argument handling.The
buildPythonCommand
function properly avoids extraneous output by only appendingargsString
when arguments exist (lines 62-64). This approach is cleaner than the trailing space issue inbuildZenCommand
.
closes #865
Summary by CodeRabbit
Tasks
Im waiting for the palyground to be merged, to get access to helper functions that are part of this for getting the default valeus for the schema