Skip to content

Conversation

@JamesHollyer
Copy link
Contributor

  • Motivation for features / changes
    This is part of an internal Large Scale Change(googlers see b/265933639).

In pr #4031 we switch from using the tools(host mode) to using exec_tools(exec mode). This was done as part of a Large Scale change(Googlers see b/163558320). This was always supposed to be a temporary measure until the host mode was switched to use python3 by default. That has now happened so we are switching back to using genrule.tools.

  • Technical description of changes
    For reference, the difference between host and exec mode:
    genrule.tools uses host mode, which ignores the py_binary.python_version attribute and always use Python 3
    genrule.exec_tools uses exec mode, which respects the py_binary.python_version attribute (unless an upstream host
    dependency forces all downstream targets to stay in host)

  • Detailed steps to verify changes work correctly (as executed by you)
    bazel clean
    bazel build tensorboard/components/tensor_widget:gen_tensor_widget_style
    bazel build tensorboard/components_polymer3/polymer:gen_plottable_style
    bazel build tensorboard/components/polymer:gen_plottable_style

@JamesHollyer JamesHollyer requested a review from bmd3k March 13, 2023 22:15
Copy link
Contributor

@bmd3k bmd3k left a comment

Choose a reason for hiding this comment

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

Should we change the other two usages in our repo, too, for consistency, even if they don't get imported internally?

https://cs.opensource.google/search?q=exec_tools&ss=tensorflow%2Ftensorboard

The usage in tensorboard/defs/defs.bzl is refactored from one of the changes in #4031.

@JamesHollyer
Copy link
Contributor Author

Should we change the other two usages in our repo, too, for consistency, even if they don't get imported internally?

https://cs.opensource.google/search?q=exec_tools&ss=tensorflow%2Ftensorboard

The usage in tensorboard/defs/defs.bzl is refactored from one of the changes in #4031.

Yea, that seems like a good idea. Done.

@JamesHollyer JamesHollyer requested a review from bmd3k March 14, 2023 19:31
@JamesHollyer JamesHollyer merged commit 6b25826 into tensorflow:master Mar 15, 2023
@katre
Copy link

katre commented Mar 15, 2023

Thanks!

yatbear pushed a commit to yatbear/tensorboard that referenced this pull request Mar 27, 2023
…low#6238)

* Motivation for features / changes
This is part of an internal Large Scale Change(googlers see
b/265933639).

In pr tensorflow#4031 we switch from using the tools(host mode) to using
exec_tools(exec mode). This was done as part of a Large Scale
change(Googlers see b/163558320). This was always supposed to be a
temporary measure until the host mode was switched to use python3 by
default. That has now happened so we are switching back to using
genrule.tools.

* Technical description of changes
For reference, the difference between host and exec mode:
genrule.tools uses host mode, which ignores the py_binary.python_version
attribute and always use Python 3
genrule.exec_tools uses exec mode, which respects the
py_binary.python_version attribute (unless an upstream host
dependency forces all downstream targets to stay in host)

* Detailed steps to verify changes work correctly (as executed by you)
bazel clean
bazel build tensorboard/components/tensor_widget:gen_tensor_widget_style
bazel build tensorboard/components_polymer3/polymer:gen_plottable_style 
bazel build tensorboard/components/polymer:gen_plottable_style
dna2github pushed a commit to dna2fork/tensorboard that referenced this pull request May 1, 2023
…low#6238)

* Motivation for features / changes
This is part of an internal Large Scale Change(googlers see
b/265933639).

In pr tensorflow#4031 we switch from using the tools(host mode) to using
exec_tools(exec mode). This was done as part of a Large Scale
change(Googlers see b/163558320). This was always supposed to be a
temporary measure until the host mode was switched to use python3 by
default. That has now happened so we are switching back to using
genrule.tools.

* Technical description of changes
For reference, the difference between host and exec mode:
genrule.tools uses host mode, which ignores the py_binary.python_version
attribute and always use Python 3
genrule.exec_tools uses exec mode, which respects the
py_binary.python_version attribute (unless an upstream host
dependency forces all downstream targets to stay in host)

* Detailed steps to verify changes work correctly (as executed by you)
bazel clean
bazel build tensorboard/components/tensor_widget:gen_tensor_widget_style
bazel build tensorboard/components_polymer3/polymer:gen_plottable_style 
bazel build tensorboard/components/polymer:gen_plottable_style
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.

3 participants