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

manager: use portable epoch time computation #1900

Merged
merged 1 commit into from
Feb 26, 2019

Conversation

wchargin
Copy link
Contributor

Summary:
Fixes #1895 by replacing the (apparently unportable) %s format
specifier with an explicit subtraction from epoch.

Test Plan:
I don’t have easy access to a Windows machine with a Bazel/TensorBoard
setup, but I verified that this expression can be evaluated by itself:

Python 3.6.8 (tags/v3.6.8:3c6b436a57, Dec 24 2018, 00:16:47) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license()" for more information.
>>> import datetime
>>> dt = datetime.datetime.now()
>>> dt.strftime("%s")
Traceback (most recent call last):
  File "<pyshell#2>", line 1, in <module>
    dt.strftime("%s")
ValueError: Invalid format string
>>> int((dt - datetime.datetime.fromtimestamp(0)).total_seconds())
1551203889

Running git grep strftime.*%s returns no matches as of this commit.

wchargin-branch: windows-strftime

Summary:
Fixes #1895 by replacing the (apparently unportable) `%s` format
specifier with an explicit subtraction from epoch.

Test Plan:
I don’t have easy access to a Windows machine with a Bazel/TensorBoard
setup, but I verified that this expression can be evaluated by itself:

```
Python 3.6.8 (tags/v3.6.8:3c6b436a57, Dec 24 2018, 00:16:47) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license()" for more information.
>>> import datetime
>>> dt = datetime.datetime.now()
>>> dt.strftime("%s")
Traceback (most recent call last):
  File "<pyshell#2>", line 1, in <module>
    dt.strftime("%s")
ValueError: Invalid format string
>>> int((dt - datetime.datetime.fromtimestamp(0)).total_seconds())
1551203889
```

Running `git grep strftime.*%s` returns no matches as of this commit.

wchargin-branch: windows-strftime
@nfelt
Copy link
Contributor

nfelt commented Feb 26, 2019

LGTM, bonus points for good use of labels.

@wchargin wchargin merged commit aa87623 into master Feb 26, 2019
@wchargin wchargin deleted the wchargin-windows-strftime branch February 26, 2019 18:28
wchargin added a commit to wchargin/tensorboard that referenced this pull request Mar 5, 2019
Summary:
Fixes tensorflow#1895 by replacing the (apparently unportable) `%s` format
specifier with an explicit subtraction from epoch.

Test Plan:
I don’t have easy access to a Windows machine with a Bazel/TensorBoard
setup, but I verified that this expression can be evaluated by itself:

```
Python 3.6.8 (tags/v3.6.8:3c6b436a57, Dec 24 2018, 00:16:47) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license()" for more information.
>>> import datetime
>>> dt = datetime.datetime.now()
>>> dt.strftime("%s")
Traceback (most recent call last):
  File "<pyshell#2>", line 1, in <module>
    dt.strftime("%s")
ValueError: Invalid format string
>>> int((dt - datetime.datetime.fromtimestamp(0)).total_seconds())
1551203889
```

Running `git grep strftime.*%s` returns no matches as of this commit.

wchargin-branch: windows-strftime
@wchargin wchargin mentioned this pull request Mar 5, 2019
wchargin added a commit that referenced this pull request Mar 6, 2019
Summary:
Fixes #1895 by replacing the (apparently unportable) `%s` format
specifier with an explicit subtraction from epoch.

Test Plan:
I don’t have easy access to a Windows machine with a Bazel/TensorBoard
setup, but I verified that this expression can be evaluated by itself:

```
Python 3.6.8 (tags/v3.6.8:3c6b436a57, Dec 24 2018, 00:16:47) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license()" for more information.
>>> import datetime
>>> dt = datetime.datetime.now()
>>> dt.strftime("%s")
Traceback (most recent call last):
  File "<pyshell#2>", line 1, in <module>
    dt.strftime("%s")
ValueError: Invalid format string
>>> int((dt - datetime.datetime.fromtimestamp(0)).total_seconds())
1551203889
```

Running `git grep strftime.*%s` returns no matches as of this commit.

wchargin-branch: windows-strftime
@mabrowning
Copy link

mabrowning commented Mar 29, 2019

Sadly:

Python 3.6.6 (v3.6.6:4cf1f54eb7, Jun 27 2018, 03:37:03) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime
>>> dt = datetime.datetime.now()
>>> int((dt - datetime.datetime.fromtimestamp(0)).total_seconds())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 22] Invalid argument
>>>

My server is set to UTC, so 0 is below the minimum time:
https://stackoverflow.com/a/45372194/381313

This works, but is gross: int((dt - datetime.datetime.fromtimestamp(86400)).total_seconds())+86400

Python 3.6.6 (v3.6.6:4cf1f54eb7, Jun 27 2018, 03:37:03) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime
>>> dt = datetime.datetime.now()
>>> ts = int((dt - datetime.datetime.fromtimestamp(86400)).total_seconds())+86400
>>> dt
datetime.datetime(2019, 3, 29, 18, 23, 6, 892661)
>>> datetime.datetime.fromtimestamp(ts)
datetime.datetime(2019, 3, 29, 18, 23, 6)

@wchargin
Copy link
Contributor Author

Yes, thank you. This is known (#2017) and fixed (#2054). If you’d like
to pull in our latest nightlies to try it out, please uninstall the
tensorboard package and then install tb-nightly.

@mabrowning
Copy link

Got it! I did go as far as checking the release notes this PR was cherry picked into... but I didn't go to check the next one. Thanks!

@wchargin
Copy link
Contributor Author

Sure thing, and thank you for reporting it!

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

Successfully merging this pull request may close these issues.

3 participants