Skip to content

uploader: include metadata in list output #2941

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

Merged
merged 11 commits into from
Nov 25, 2019

Conversation

wchargin
Copy link
Contributor

@wchargin wchargin commented Nov 15, 2019

Summary:
This commit teaches the uploader to display experiment metadata included
in StreamExperiments responses by supported servers. For servers
without this support, the change is a backward-compatible no-op.

The format is intentionally undocumented and not under any compatibility
guarantees, but is designed to be easily parseable for ad hoc usage. For
instance, this simple one-liner finds experiments with lots of points so
that the user can delete them:

tensorboard dev list |
    awk '$1 == "Id" { id = $2 } $1 == "Scalars" && $2 > 1000 { print id }'

Test Plan:
Running against current prod, which does not yet support the new RPCs,
the behavior is unchanged:

$ bazel run //tensorboard -- dev list
https://tensorboard.dev/experiment/IAVF94GPSWWBTvonQe4kgQ/
https://tensorboard.dev/experiment/LiQNYkOHRSGEWj42xtgtjA/
<snip>
Total: 12 experiment(s)

Running against a local server with support for the new RPCs, we see
lots of additional data (tested on both Linux and Windows):

$ bazel run //tensorboard -- dev --origin http://localhost:8080 --grpc_creds_type ssl_dev list
http://localhost:8080/experiment/WtPawgPIQXi2SZ1fQszOFA/
	Id         WtPawgPIQXi2SZ1fQszOFA
	Created    2019-11-25 10:30:18 (23 seconds ago)
	Updated    2019-11-25 10:30:39 (just now)
	Scalars    18814
	Runs       21
	Tags       7
http://localhost:8080/experiment/jD7Qc7l6S8Wy5gWKYTAHOA/
	Id         jD7Qc7l6S8Wy5gWKYTAHOA
	Created    2019-11-13 18:32:06
	Updated    2019-11-13 18:32:06
	Scalars    0
	Runs       0
	Tags       0
http://localhost:8080/experiment/do8uvvEOSNWOUEANmQIprQ/
	Id         do8uvvEOSNWOUEANmQIprQ
	Created    2019-11-13 18:15:25
	Updated    2019-11-13 18:15:37
	Scalars    3208
	Runs       8
	Tags       4
<snip>
Total: 9 experiment(s)

Also tested that the tensorboard dev export service still works
against both old and new servers.

wchargin-branch: uploader-list-metadata

Summary:
This extends the `StreamExperiments` RPC such that the client can
specify a set of additional metadata fields that the server should
include, like “creation time” or “number of scalar points”.

The format is both forward- and backward-compatible. Servers are
expected to send responses with both `experiment_ids` and `experiments`
until we drop support for clients that do not support `experiments`, at
which point they need only send `experiments`.

Test Plan:
Unit test added to simulate the future behavior of servers.

wchargin-branch: streamexperiments-metadata
wchargin-branch: streamexperiments-metadata
wchargin-source: a9770c6a0a6f1a56a96721b4437ca33bb15974d0
wchargin-branch: streamexperiments-metadata
wchargin-source: a9770c6a0a6f1a56a96721b4437ca33bb15974d0
Summary:
This commit teaches the uploader to display experiment metadata included
in `StreamExperiments` responses by supported servers. For servers
without this support, the change is a backward-compatible no-op.

The format is explicitly undocumented and not subject to compatibility
guarantees, but is designed to be easily parseable for ad hoc usage. For
instance, this simple one-liner finds experiments with lots of points so
that the user can delete them:

```
tensorboard dev list |
    awk '$1 == "Id" { id = $2 } $1 == "Scalars" && $2 > 1000 { print id }'
```

Test Plan:
Running against current prod, which does not yet support the new RPCs,
the behavior is unchanged:

```
$ bazel run //tensorboard -- dev list
https://tensorboard.dev/experiment/IAVF94GPSWWBTvonQe4kgQ/
https://tensorboard.dev/experiment/LiQNYkOHRSGEWj42xtgtjA/
<snip>
Total: 12 experiment(s)
```

Running against a local server with support for the new RPCs, we see
lots of additional data:

```
$ bazel run //tensorboard -- dev --origin http://localhost:8080 --grpc_creds_type ssl_dev list
http://localhost:8080/experiment/qIONOgA7RQKJTDO1llUMzA/
	Id         qIONOgA7RQKJTDO1llUMzA
	Created    5 seconds ago
	Updated    just now
	Scalars    250
	Runs       5
	Tags       5
http://localhost:8080/experiment/CV0PT3C6RKS3Vdg6D7zt1Q/
	Id         CV0PT3C6RKS3Vdg6D7zt1Q
	Created    23:42:10 ago
	Updated    23:42:10 ago
	Scalars    250
	Runs       5
	Tags       5
http://localhost:8080/experiment/do8uvvEOSNWOUEANmQIprQ/
	Id         do8uvvEOSNWOUEANmQIprQ
	Created    2019-11-13 18:15:25
	Updated    2019-11-13 18:15:37
	Scalars    3208
	Runs       8
	Tags       4
<snip>
Total: 9 experiment(s)
```

wchargin-branch: uploader-list-metadata
wchargin-source: 8d29c17ea904e092be8cf3e2bf07180aa8777c54
wchargin-branch: streamexperiments-metadata
wchargin-source: eb73c9b8d1d48e3a0830ac4c6793a953e75e9b62
wchargin-branch: uploader-list-metadata
wchargin-source: 3943f83b2b2f1d7d55949f027abfe718a96dc60c
wchargin-branch: uploader-list-metadata
wchargin-source: 3943f83b2b2f1d7d55949f027abfe718a96dc60c
@wchargin wchargin requested a review from nfelt November 20, 2019 07:52
@wchargin wchargin changed the base branch from wchargin-streamexperiments-metadata to master November 21, 2019 23:36
wchargin-branch: uploader-list-metadata
wchargin-source: 148b022bc8ddd2dc220511c856560bf43aa5b69d

# Conflicts:
#	tensorboard/uploader/exporter.py
#	tensorboard/uploader/exporter_test.py
wchargin-branch: uploader-list-metadata
wchargin-source: 148b022bc8ddd2dc220511c856560bf43aa5b69d
@wchargin wchargin requested review from nfelt and removed request for nfelt November 21, 2019 23:37
Copy link
Contributor

@nfelt nfelt left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe worth confirming output LGT @GalOshri too if you didn't already.


ago = datetime.datetime.now().replace(microsecond=0) - dt
if ago < datetime.timedelta(seconds=5):
return "just now"
Copy link
Contributor

Choose a reason for hiding this comment

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

How would you feel about providing the exact timestamp in all cases and maybe just add the human-friendly interpretations of recent times in parentheses after the timestamp or something? Maybe just my pet peeve, but as a user I'm often a little annoyed when there's a humanized timestamp with no way to access the exact value (e.g. via the title text or something).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree; uncharacteristic of me to omit that. Will change.

Copy link
Contributor Author

@wchargin wchargin left a comment

Choose a reason for hiding this comment

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

Thanks! @GalOshri, please see output in PR description and let me know
if you have any requests. We can also always change the output format
later, though doing so will require a client-side release rather than
just a server-side deploy.


ago = datetime.datetime.now().replace(microsecond=0) - dt
if ago < datetime.timedelta(seconds=5):
return "just now"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree; uncharacteristic of me to omit that. Will change.

@GalOshri
Copy link
Contributor

LGTM with the change mentioned above regarding "Created" and "Updated" formats. Consistency would be useful, and the second example ("23:42:10 ago") is somewhat confusing.

Since we are not including a message on "you are using X% of your quota", let's make sure we have a bug to track that for the future?

@GalOshri
Copy link
Contributor

Also, thank you for adding this! This is super helpful.

wchargin-branch: uploader-list-metadata
wchargin-source: 8d0f92c7218a8eaa8442fcefed2113ed9161b235
wchargin-branch: uploader-list-metadata
wchargin-source: 8d0f92c7218a8eaa8442fcefed2113ed9161b235
@wchargin
Copy link
Contributor Author

Moved time formatting logic into a utility module so that it can be
tested. See updated PR description for revised output form:

	Created    2019-11-25 10:30:18 (23 seconds ago)
	Updated    2019-11-25 10:30:39 (just now)

Tested on both Linux and Windows against both prod and a dev API server
that supports the new functionality.

@wchargin
Copy link
Contributor Author

Since we are not including a message on "you are using X% of your
quota",

Correct; the backend doesn’t send down that information, and it would be
restrictive to hard-code a value (doesn’t give much flexibility to
change it later without a new uploader version).

let's make sure we have a bug to track that for the future?

Opened #2969.

@wchargin wchargin merged commit ae2aaa8 into master Nov 25, 2019
@wchargin wchargin deleted the wchargin-uploader-list-metadata branch November 25, 2019 20:55
wchargin added a commit to wchargin/tensorboard that referenced this pull request Nov 25, 2019
Summary:
This commit teaches the uploader to display experiment metadata included
in `StreamExperiments` responses by supported servers. For servers
without this support, the change is a backward-compatible no-op.

The format is intentionally undocumented and not under any compatibility
guarantees, but is designed to be easily parseable for ad hoc usage. For
instance, this simple one-liner finds experiments with lots of points so
that the user can delete them:

```
tensorboard dev list |
    awk '$1 == "Id" { id = $2 } $1 == "Scalars" && $2 > 1000 { print id }'
```

Test Plan:
Running against current prod, which does not yet support the new RPCs,
the behavior is unchanged:

```
$ bazel run //tensorboard -- dev list
https://tensorboard.dev/experiment/IAVF94GPSWWBTvonQe4kgQ/
https://tensorboard.dev/experiment/LiQNYkOHRSGEWj42xtgtjA/
<snip>
Total: 12 experiment(s)
```

Running against a local server with support for the new RPCs, we see
lots of additional data (tested on both Linux and Windows):

```
$ bazel run //tensorboard -- dev --origin http://localhost:8080 --grpc_creds_type ssl_dev list
http://localhost:8080/experiment/WtPawgPIQXi2SZ1fQszOFA/
	Id         WtPawgPIQXi2SZ1fQszOFA
	Created    2019-11-25 10:30:18 (23 seconds ago)
	Updated    2019-11-25 10:30:39 (just now)
	Scalars    18814
	Runs       21
	Tags       7
http://localhost:8080/experiment/jD7Qc7l6S8Wy5gWKYTAHOA/
	Id         jD7Qc7l6S8Wy5gWKYTAHOA
	Created    2019-11-13 18:32:06
	Updated    2019-11-13 18:32:06
	Scalars    0
	Runs       0
	Tags       0
http://localhost:8080/experiment/do8uvvEOSNWOUEANmQIprQ/
	Id         do8uvvEOSNWOUEANmQIprQ
	Created    2019-11-13 18:15:25
	Updated    2019-11-13 18:15:37
	Scalars    3208
	Runs       8
	Tags       4
<snip>
Total: 9 experiment(s)
```

Also tested that the `tensorboard dev export` service still works
against both old and new servers.

wchargin-branch: uploader-list-metadata
@wchargin wchargin mentioned this pull request Nov 25, 2019
wchargin added a commit that referenced this pull request Nov 25, 2019
Summary:
This commit teaches the uploader to display experiment metadata included
in `StreamExperiments` responses by supported servers. For servers
without this support, the change is a backward-compatible no-op.

The format is intentionally undocumented and not under any compatibility
guarantees, but is designed to be easily parseable for ad hoc usage. For
instance, this simple one-liner finds experiments with lots of points so
that the user can delete them:

```
tensorboard dev list |
    awk '$1 == "Id" { id = $2 } $1 == "Scalars" && $2 > 1000 { print id }'
```

Test Plan:
Running against current prod, which does not yet support the new RPCs,
the behavior is unchanged:

```
$ bazel run //tensorboard -- dev list
https://tensorboard.dev/experiment/IAVF94GPSWWBTvonQe4kgQ/
https://tensorboard.dev/experiment/LiQNYkOHRSGEWj42xtgtjA/
<snip>
Total: 12 experiment(s)
```

Running against a local server with support for the new RPCs, we see
lots of additional data (tested on both Linux and Windows):

```
$ bazel run //tensorboard -- dev --origin http://localhost:8080 --grpc_creds_type ssl_dev list
http://localhost:8080/experiment/WtPawgPIQXi2SZ1fQszOFA/
	Id         WtPawgPIQXi2SZ1fQszOFA
	Created    2019-11-25 10:30:18 (23 seconds ago)
	Updated    2019-11-25 10:30:39 (just now)
	Scalars    18814
	Runs       21
	Tags       7
http://localhost:8080/experiment/jD7Qc7l6S8Wy5gWKYTAHOA/
	Id         jD7Qc7l6S8Wy5gWKYTAHOA
	Created    2019-11-13 18:32:06
	Updated    2019-11-13 18:32:06
	Scalars    0
	Runs       0
	Tags       0
http://localhost:8080/experiment/do8uvvEOSNWOUEANmQIprQ/
	Id         do8uvvEOSNWOUEANmQIprQ
	Created    2019-11-13 18:15:25
	Updated    2019-11-13 18:15:37
	Scalars    3208
	Runs       8
	Tags       4
<snip>
Total: 9 experiment(s)
```

Also tested that the `tensorboard dev export` service still works
against both old and new servers.

wchargin-branch: uploader-list-metadata
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.

4 participants