-
Notifications
You must be signed in to change notification settings - Fork 8
fix: workflow output path schedule mode #745
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: main
Are you sure you want to change the base?
Changes from 4 commits
e7faea8
989bdd4
0e76789
d01931c
c10ff41
00bc161
6f39e65
b257dcc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| import asyncio | ||
| import glob | ||
| import os | ||
| import re | ||
| from datetime import timedelta | ||
| from functools import wraps | ||
| from typing import Any, Awaitable, Callable, List, Optional, TypeVar, cast | ||
|
|
@@ -22,6 +23,9 @@ | |
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| # Compiled regex pattern for removing timestamp suffix from workflow IDs | ||
| TIMESTAMP_PATTERN = re.compile(r"-\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$") | ||
|
|
||
|
|
||
| F = TypeVar("F", bound=Callable[..., Awaitable[Any]]) | ||
|
|
||
|
|
@@ -63,6 +67,7 @@ def build_output_path() -> str: | |
| """Build a standardized output path for workflow artifacts. | ||
|
|
||
| This method creates a consistent output path format across all workflows using the WORKFLOW_OUTPUT_PATH_TEMPLATE constant. | ||
| For scheduled workflows, it removes any timestamp suffix from the workflow_id to ensure consistent output paths. | ||
|
|
||
| Returns: | ||
| str: The standardized output path. | ||
|
|
@@ -71,9 +76,19 @@ def build_output_path() -> str: | |
| >>> build_output_path() | ||
| "artifacts/apps/appName/workflows/wf-123/run-456" | ||
| """ | ||
| # Sanitize workflow_id to remove any schedule/timestamp suffix | ||
| raw_workflow_id = get_workflow_id() | ||
|
|
||
| # Remove timestamp suffix (e.g., '-YYYY-MM-DDTHH:MM:SSZ') if present | ||
| sanitized_workflow_id = TIMESTAMP_PATTERN.sub("", raw_workflow_id) | ||
|
|
||
| # Fallback to raw workflow_id if sanitization results in empty string | ||
| if not sanitized_workflow_id: | ||
| sanitized_workflow_id = raw_workflow_id | ||
|
||
|
|
||
| return WORKFLOW_OUTPUT_PATH_TEMPLATE.format( | ||
| application_name=APPLICATION_NAME, | ||
| workflow_id=get_workflow_id(), | ||
| workflow_id=sanitized_workflow_id, | ||
| run_id=get_workflow_run_id(), | ||
| ) | ||
|
|
||
|
|
||
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.
let's keep this variable localized as it's only used by a a single method as of now.
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.
I implemented that initially, but the copilot suggested me otherwise, what should i go with?