Add request and response for WorkflowGet API #1076
Conversation
WalkthroughIntroduces two Thrift structs in api/thrift/orchestration.thrift: WorkflowGetRequest and WorkflowGetResponse with 12 optional fields including WorkflowStatus. They are placed after WorkflowStatusRequest and before NodeStatusRequest. An extra closing brace was added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrchestrationAPI
Client->>OrchestrationAPI: WorkflowGetRequest(workflowId?)
OrchestrationAPI-->>Client: WorkflowGetResponse(runId, conf*, mode, partitions, branch, user, plan, submissionTime, forceRecompute, status)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
api/thrift/orchestration.thrift (1)
184-186: Is workflowId actually optional?Semantically required for a GET-by-id. If you keep it optional for backward-compat, validate server-side and document it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
api/thrift/orchestration.thrift(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Test non-spark modules on scala 2.12
api/thrift/orchestration.thrift
[error] 203-203: Thrift code generation failed due to syntax error: last token was '}' at line 203. Parser error during include pass. Thrift command failed: /usr/local/bin/thrift --gen 'java:generated_annotations=undated' -out bazel-out/k8-fastbuild/bin/api/thrift_gen_orchestration api/thrift/orchestration.thrift
🪛 GitHub Actions: Test Python
api/thrift/orchestration.thrift
[error] 203-203: Thrift syntax error: (last token was '}') in orchestration.thrift:203. Thrift generation failed. Command: thrift --gen py -out api/python/ api/thrift/orchestration.thrift
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: scala_compile_fmt_fix
- GitHub Check: streaming_tests
- GitHub Check: fetcher_tests
- GitHub Check: join_tests
- GitHub Check: analyzer_tests
| struct WorkflowGetResponse { | ||
| 1: optional string runId | ||
| 2: optional string confName |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use workflowId (not runId) for workflow identity
API elsewhere uses workflowId (WorkflowStartResponse, WorkflowStatusResponse, WorkflowResponse). runId is used for node step runs; using it here is confusing. Rename now before clients adopt it.
- 1: optional string runId
+ 1: optional string workflowId📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| struct WorkflowGetResponse { | |
| 1: optional string runId | |
| 2: optional string confName | |
| struct WorkflowGetResponse { | |
| 1: optional string workflowId | |
| 2: optional string confName | |
| // …other fields… | |
| } |
🤖 Prompt for AI Agents
In api/thrift/orchestration.thrift around lines 188–190, the WorkflowGetResponse
struct uses the field name runId which is inconsistent with the rest of the API;
rename field 1 from runId to workflowId (keep id 1 and optional string type) and
update any local references in this Thrift file to use workflowId instead of
runId; after changing the IDL, regenerate/update any generated client/server
code and update call sites/tests to refer to WorkflowGetResponse.workflowId so
the API consistently uses workflowId for workflow identity.
| } | ||
|
|
There was a problem hiding this comment.
Remove stray closing brace to fix Thrift parse error
Extraneous '}' breaks codegen (see pipeline error at line 203). Remove it.
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } |
🧰 Tools
🪛 GitHub Actions: Test non-spark modules on scala 2.12
[error] 203-203: Thrift code generation failed due to syntax error: last token was '}' at line 203. Parser error during include pass. Thrift command failed: /usr/local/bin/thrift --gen 'java:generated_annotations=undated' -out bazel-out/k8-fastbuild/bin/api/thrift_gen_orchestration api/thrift/orchestration.thrift
🪛 GitHub Actions: Test Python
[error] 203-203: Thrift syntax error: (last token was '}') in orchestration.thrift:203. Thrift generation failed. Command: thrift --gen py -out api/python/ api/thrift/orchestration.thrift
🤖 Prompt for AI Agents
In api/thrift/orchestration.thrift around lines 203 to 204, there's an
extraneous closing brace '}' that causes a Thrift parse/codegen error; remove
this stray '}' so the file's braces match (verify surrounding
struct/enum/service blocks) and run the Thrift linter/codegen to confirm the
syntax error is resolved.
Summary
Checklist
Summary by CodeRabbit