feature to write the name of artifact with custom and dynamic inputs #234
Conversation
…namic inputs Signed-off-by: Krishna Mohan <[email protected]>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
krishna9358
changed the title
Signed-off-by: Krishna Mohan <[email protected]>
Review FeedbackIssue 1:
|
| return template | ||
| .replace(/\{\{run_id\}\}/gi, context.runId) | ||
| .replace(/\{\{timestamp\}\}/gi, timestamp) | ||
| .replace(/\{\{dataset\}\}/gi, 'default') |
There was a problem hiding this comment.
This placeholder is hardcoded to always return "default". Either remove this placeholder or properly implement it by adding dataset to the execution context.
| .replace(/\{\{run_id\}\}/gi, context.runId) | ||
| .replace(/\{\{timestamp\}\}/gi, timestamp) | ||
| .replace(/\{\{dataset\}\}/gi, 'default') | ||
| .replace(/\{\{task\}\}/gi, context.componentRef) |
There was a problem hiding this comment.
Misleading name: this returns the component reference ID (node ID in workflow), not a task name. Consider renaming to node_id or component_ref for clarity.
| * Supported placeholders: | ||
| * - {{run_id}} - Current run ID | ||
| * - {{timestamp}} - Unix timestamp in milliseconds | ||
| * - {{dataset}} - Dataset name (if available) |
There was a problem hiding this comment.
The documentation says "Dataset name (if available)" but this actually returns the hardcoded string "default". Please either remove this placeholder or implement it properly.
| * - {{run_id}} - Current run ID | ||
| * - {{timestamp}} - Unix timestamp in milliseconds | ||
| * - {{dataset}} - Dataset name (if available) | ||
| * - {{task}} - Task name (if available) |
There was a problem hiding this comment.
The documentation says "Task name (if available)" but this actually returns the component reference ID. Please update the documentation or rename the placeholder.
|
|
||
| // Extract a shorter run name from runId (last segment after last hyphen, or first 8 chars) | ||
| const runIdParts = context.runId.split('-'); | ||
| const runName = runIdParts.length > 1 ? runIdParts.slice(-1)[0] : context.runId.slice(0, 8); |
There was a problem hiding this comment.
This logic is confusing and unpredictable - it takes the last segment after a hyphen, or the first 8 characters of the runId. Consider removing this placeholder or adding an actual run name to the execution context.
Signed-off-by: Krishna Mohan <[email protected]>
Signed-off-by: Krishna Mohan <[email protected]>
Signed-off-by: Krishna Mohan <[email protected]>
|
Summary
|
…namic inputs
Summary
Changes
Testing
bun run testbun run lintbun run typecheckDocumentation
docs/guide.md) or checked that no updates are needed..ailogs when applicable.