-
Notifications
You must be signed in to change notification settings - Fork 29
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
Separate datetime components in logfile names with punctuation #1445
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1445 +/- ##
=======================================
Coverage 88.61% 88.62%
=======================================
Files 77 77
Lines 10563 10563
=======================================
+ Hits 9360 9361 +1
+ Misses 1203 1202 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
dandi/cli/command.py
Outdated
@@ -99,7 +99,7 @@ def main(ctx, log_level, pdb=False): | |||
|
|||
logdir = platformdirs.user_log_dir("dandi-cli", "dandi") | |||
logfile = os.path.join( | |||
logdir, f"{datetime.now(timezone.utc):%Y%m%d%H%M%SZ}-{os.getpid()}.log" | |||
logdir, f"{datetime.now(timezone.utc):%Y.%m.%d.%H.%M.%SZ}-{os.getpid()}.log" |
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.
so instead of
20240503020100Z-1853999.log
it would become
2024.05.03.02.01.00Z-1853999.log
which also doesn't help in separating the date from time - IMHO the most important one for readability. What about middle ground of just
logdir, f"{datetime.now(timezone.utc):%Y.%m.%d.%H.%M.%SZ}-{os.getpid()}.log" | |
logdir, f"{datetime.now(timezone.utc):%Y%m%d.%H%M%SZ}-{os.getpid()}.log" |
ie.
20240503.020100Z-1853999.log
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.
@yarikoptic I think just separating the individual components is the most important part for readability. How about keeping the component separators but changing the date-time separator to a hyphen, e.g., 2024.05.03-02.01.00Z-1853999.log
?
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.
looks better! one more possible iteration: in duct (attn @asmacdo ) ATM we take iso8601 but replacing :
with -
as well to avoid :
in filename (but also lacking Z
for timezone depiction)... so it then could be 2024-05-03T02-01-00Z-1853999.log
. Frankly using .
makes it cleaner. So may be let's go with this last iteration and consider doing the same in duct
@asmacdo
@yarikoptic Can this be merged now? |
🚀 PR was released in |
The logfile names are currently too hard to read; the names need to be broken up more.