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

Use --timestamp in placeholders if given #5472

Merged

Conversation

milkey-mouse
Copy link
Contributor

Fixes #5189. {now} and {utcnow} will take the value of --timestamp if given. I also added a test for this functionality.

@ThomasWaldmann
Copy link
Member

Looks good. Some questions:

  • is location the only thing using the placeholders? maybe check which callers of replace_placeholders are relevant.
  • what tzoffset does your test system have?

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Nov 3, 2020

Also: can you please rebase on current master and push -f, so that the CI via github workflows kicks in?

@milkey-mouse milkey-mouse force-pushed the timestamp-aware-placeholders branch from af4204b to 3644bd1 Compare November 3, 2020 21:00
@milkey-mouse
Copy link
Contributor Author

milkey-mouse commented Nov 3, 2020

Location and some tests are the only things using replace_placeholders, aside from this part in remote.py:

borg/src/borg/remote.py

Lines 687 to 689 in 47e96bc

remote_path = args.remote_path or os.environ.get('BORG_REMOTE_PATH', 'borg')
remote_path = replace_placeholders(remote_path)
return env_vars + [remote_path, 'serve'] + opts

I'm not sure if getting the value from --timestamp makes sense here; I don't think so.

My system has tzoffset -08:00 but that shouldn't matter as all the times involved are UTC. I tried changing my system's timezone and rerunning the tests, everything worked the same.

@ThomasWaldmann ThomasWaldmann merged commit dea3f01 into borgbackup:master Nov 3, 2020
@ThomasWaldmann
Copy link
Member

Thanks! Would you like to do a backport to 1.1-maint, so it gets into next 1.1.x release?

@ThomasWaldmann
Copy link
Member

https://github.com/borgbackup/borg/pull/5472/checks?check_run_id=1349770343

hmm, can we find out why ubuntu / py37 is hanging?

@milkey-mouse milkey-mouse deleted the timestamp-aware-placeholders branch November 4, 2020 18:16
@milkey-mouse
Copy link
Contributor Author

Sure, I'll backport.

@milkey-mouse
Copy link
Contributor Author

I can't reproduce the py37 hang on my local machine, although I'm not running Ubuntu. But the 1.1 backport passed CI just fine.

@milkey-mouse milkey-mouse mentioned this pull request Nov 5, 2020
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.

--timestamp-aware {now}/{utcnow} placeholders
2 participants