KEMBAR78
Refactor logic common to send Tensors and Scalars by bmd3k · Pull Request #3537 · tensorflow/tensorboard · GitHub
Skip to content

Conversation

@bmd3k
Copy link
Contributor

@bmd3k bmd3k commented Apr 22, 2020

  • Motivation for features / changes

The future _TensorBatchedRequestSender behaves similarly to the existing _ScalarBatchedRequestSender. This change refactors some of the less-trivial logic out from _ScalarBatchedRequestSender to be reused in _TensorBatchedRequestSender.

  • Technical description of changes

Refactor logic for managing request byte budget into its own class and for pruning empty runs and tags into its own function.

One test is improved to use mocked _ByteBudgetManager.

bmd3k added 3 commits April 20, 2020 11:49
Identify tests in uploader_test.py that seem to specifically target
logic in _ScalarBatchedRequestSender and rewrite them to only use
_ScalarBatchedRequestSender and none of the layers above it.

This is a useful exercise for the work on _TensorBatchedRequestSender so
we can more easily identify which tests need to be written for it.

We could also consider refactoring _ScalarBatchedRequestSender to its
own file.
This refactors logic out of _ScalarBatchedRequestSender that will be
reused in the future _TensorBatchRequestSender. Specifically we refactor
logic for managing byte budget for requests and for pruning empty runs
and tags.

One test is refactored to use mocked _ByteBudgetManager.
@bmd3k bmd3k marked this pull request as ready for review April 22, 2020 16:11
@bmd3k bmd3k requested review from bileschi and davidsoergel April 22, 2020 18:04
Copy link
Member

@davidsoergel davidsoergel left a comment

Choose a reason for hiding this comment

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

This would be a great cleanup even if you didn't need to reuse the _ByteBudgetManager. Nice, thanks!

@bmd3k bmd3k merged commit d93c457 into tensorflow:master Apr 23, 2020
caisq pushed a commit to caisq/tensorboard that referenced this pull request May 19, 2020
The future _TensorBatchedRequestSender behaves similarly to the existing _ScalarBatchedRequestSender. This change refactors some of the less-trivial logic out from _ScalarBatchedRequestSender to be reused in _TensorBatchedRequestSender.

Refactor logic for managing request byte budget into its own class and for pruning empty runs and tags into its own function.

One test is improved to use mocked _ByteBudgetManager.
caisq pushed a commit that referenced this pull request May 27, 2020
The future _TensorBatchedRequestSender behaves similarly to the existing _ScalarBatchedRequestSender. This change refactors some of the less-trivial logic out from _ScalarBatchedRequestSender to be reused in _TensorBatchedRequestSender.

Refactor logic for managing request byte budget into its own class and for pruning empty runs and tags into its own function.

One test is improved to use mocked _ByteBudgetManager.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants