-
Notifications
You must be signed in to change notification settings - Fork 5
feat: replace support of "datetime_filesafe" format field in output file prefix format string with "datetime" #375
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?
Conversation
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.
Pull request overview
This PR replaces the datetime_filesafe format field with datetime in the output file prefix format string throughout the codebase. The change maintains backwards compatibility by still accepting datetime_filesafe in the LogPaths.create() method while promoting datetime as the new standard. The actual datetime format remains unchanged (still file-safe: %Y.%m.%dT%H.%M.%S).
Changes:
- Updated default
DUCT_OUTPUT_PREFIXto use{datetime}instead of{datetime_filesafe} - Modified
LogPaths.create()to support both{datetime}and{datetime_filesafe}format fields for backwards compatibility - Updated documentation in CLI help text and README to reflect the new
{datetime}field name
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/con_duct/duct_main.py | Updated default prefix constant and modified LogPaths.create() to accept both datetime and deprecated datetime_filesafe fields |
| src/con_duct/cli.py | Updated CLI help text to remove reference to datetime_filesafe and show only datetime as a supported format field |
| README.md | Updated documentation examples and help text to use {datetime} instead of {datetime_filesafe} |
| test/duct_main/test_log_paths.py | Updated test to use new {datetime} format field instead of {datetime_filesafe} |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b2e52bc to
ebe4aac
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #375 +/- ##
=======================================
Coverage 91.63% 91.63%
=======================================
Files 15 15
Lines 1112 1112
Branches 138 138
=======================================
Hits 1019 1019
Misses 70 70
Partials 23 23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
conflicts came up |
|
yep we'll need to rebase (i moved this). either I will do or @candleindark on Friday? |
ebe4aac to
9e2a15f
Compare
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ile prefix format string with "datetime"
c3ee421 to
8c7b8cc
Compare
for more information, see https://pre-commit.ci
|
@asmacdo Have you seen the error at https://github.com/con/duct/actions/runs/21525492435/job/62027909892?pr=375 before? It seems to be an unrelated issue.
It can be an intermittent failure, but I can't rerun tests in this repo to find out if that's the case. |
|
I couldn't reproduce the error on my own Mac, but my Mac is not an Intel Mac though. |
|
@candleindark yep I think this one just needs a rerunflake back off #384 |
|
Reran the failing test, and it still failed, so I've added the backoff logic to that one. #394 after we merge that we should be good here. |

This PR closes #304