Skip to content
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

Allow specifying UTC vs local timezone as program argument #77

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jbnance
Copy link
Contributor

@jbnance jbnance commented Dec 22, 2017

No description provided.

@@ -209,7 +211,7 @@ do_snapshots () # properties, flags, snapname, oldglob, [targets...]

GETOPT=$(getopt \
--longoptions=default-exclude,dry-run,fast,skip-scrub,recursive \
--longoptions=event:,keep:,label:,prefix:,sep: \
--longoptions=event:,keep:,label:,prefix:,local-tz:,sep: \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The : after local-tz needs to be removed because this option doesn't take an argument. I'd also suggest moving it down in to the next line.

DATE=$(date --utc +%F-%H%M)
# If the --local-tz flag is set use the system's timezone.
# Otherwise, the default is to use UTC.
if [ -n $"opt_local_tz" ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $ should be moved inside the quotes.

@jbnance
Copy link
Contributor Author

jbnance commented Dec 27, 2017

Fixed - thank you.

Copy link

@UweSauter UweSauter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opt_local_tz='' should be inserted after line 44 in zfs-auto-snapshot.sh

@jbnance
Copy link
Contributor Author

jbnance commented Feb 13, 2018

I added opt_local_tz default as requested. Thank you for pointing it out.

@marcopaganini
Copy link

Any plans to merge this? I'd really like to keep my snapshots in local time.

@Lynxy
Copy link

Lynxy commented Mar 7, 2019

How has this not been merged yet? I recently started using zfs-auto-snapshot and have found it really annoying that the snapshot names are all in UTC without any mention of UTC at all!

For me times are off by 5 hours. Imagine I have this script set to run at midnight, but something bad happened at 11pm. The next morning I catch the error and want to rollback snapshots.. oh but wait the most recent one is labeled 5am, well after the bad thing occurred at 11pm, guess I need to rollback even further. How is this acceptable?

I'll be manually patching my local script with this PR but strongly urge that this PR be accepted for the sanity of users everywhere.

@jrmiller82
Copy link

Please merge this for the love of all of us who don't want to do UTC calculations on our snapshots. :)

@atomicwrites
Copy link

Any plans to merge this?

@tux314159
Copy link

Please merge this, it's rather troublesome having to add 8 hours to timestamps manually every time...

@gdi2k
Copy link

gdi2k commented Dec 10, 2020

Would also love to see this merged. We give end users access to their snapshots; confusion abounds over time stamps not matching local time.

@alkisg
Copy link

alkisg commented Dec 10, 2020

A workaround is to use faketime:

$ crontab -e

PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin

10,20,30,40,50 * * * * faketime "+8 hours" zfs-auto-snapshot -q -g --label=10minutely --keep=24 //
0 1-23 * * * faketime "+8 hours" zfs-auto-snapshot -q -g --label=hourly --keep=24 //
0 0 2-7,9-14,16-21,22-31 * * faketime "+8 hours" zfs-auto-snapshot -q -g --label=daily --keep=7 //
0 0 8,15,22 * * faketime "+8 hours" zfs-auto-snapshot -q -g --label=weekly --keep=4 //
0 0 1 * * faketime "+8 hours" zfs-auto-snapshot -q -g --label=monthly --keep=4 //

@Guiorgy
Copy link

Guiorgy commented Aug 24, 2024

Made a fork to use this and other PRs, and while at it, I checked other forks to see if there's anything interesting to gather. It's funny that most of them were simple forks that removed the -u (UTC) flag 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.