-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Add instructions to configure Bazel for Python 2 #3738
Conversation
Welcome @praseodym! |
Hi @praseodym. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/assign @stevekuznetsov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ok-to-test
@fejta @timothysc @stevekuznetsov could I get a lgtm (or some feedback) on this PR? Thanks! |
`/usr/bin/env python` must be python2 in order for all the Bazel commands listed | ||
below to succeed. | ||
Instructions for installing Bazel can be found [here][bazel-install]. | ||
Note that Bazel currently does not work with Python 3, which means that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not believe in the accuracy of this statement. Why do you believe it?
If it is true, what issues should we track (please link to them) that will help us understand that this documentation is stale.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bazelbuild/bazel#8443 (referenced in the PR description too), which apparently has been fixed now. I’m not sure if something needs to be done to get that fix into Kubernetes too (how does the release tooling determine which Bazel version to use?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bazel supports python3 via python toolchain by default since bazel 0.27.0. Here is a good FAQ as well as migration steps (bazelbuild/bazel#7899). I suggest either cherrypicking useful statements from that guide or linking to it directly.
--
Moreover, the default python_version
for bazel was changed to python3 in 0.25.0 (bazelbuild/bazel#7359) and --python_path
is deprecated as of 0.27.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I've rewritten the paragraph and included a link to the FAQ as well.
/assign @clarketm |
`/usr/bin/env python` must be python2 in order for all the Bazel commands listed | ||
below to succeed. | ||
Instructions for installing Bazel can be found [here][bazel-install]. | ||
Note that Bazel currently does not work with Python 3, which means that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bazel supports python3 via python toolchain by default since bazel 0.27.0. Here is a good FAQ as well as migration steps (bazelbuild/bazel#7899). I suggest either cherrypicking useful statements from that guide or linking to it directly.
--
Moreover, the default python_version
for bazel was changed to python3 in 0.25.0 (bazelbuild/bazel#7359) and --python_path
is deprecated as of 0.27.0
aadc1e7
to
a92a970
Compare
a92a970
to
b1b1be8
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: clarketm, fejta, praseodym The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR adds instructions to configure Bazel with Python 2. I removed the explicit reference to bazelbuild/rules_docker#454 because now that it has been fixed,
make_deb.py
is broken (bazelbuild/bazel#8443) and Python 3 still doesn't work.I also turned all links in the paragraph into references, which makes the source a little more readable.