Inconsistency: Implicit dependencies for a pip package used if already installed, needs to be explicitly specified as requirement otherwise

[ghostwriternr]

TL;DR: Does rules_python expect that all explicit and implicit dependencies be specified as a 'requirement'?

I was using google-auth in my project today when I noticed this issue.

SCENARIO 1:

Instance with requests Python package installed in the current environment.
My requirements.txt:

...
google-auth>=1.0.0

My BUILD file:

...
py_library(
    name = "methods",
    srcs = ["methods.py"],
    deps = [
        requirement('google-auth'),
    ],
)
...

Observation

My Bazel test failed with the following stacktrace, complaining about not finding the requests library:

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //src/python/grpcio_tests/tests/interop:_secure_intraop_test
-----------------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/.cache/bazel/_bazel_root/ec9ee79d770c6b7183036e592ac23980/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/pyt
hon/grpcio_tests/tests/interop/_secure_intraop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests/interop/_secure_intraop_
test.py", line 21, in <module>
    from tests.interop import _intraop_test_case
  File "/root/.cache/bazel/_bazel_root/ec9ee79d770c6b7183036e592ac23980/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/pyt
hon/grpcio_tests/tests/interop/_secure_intraop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests/interop/_intraop_test_ca
se.py", line 16, in <module>
    from tests.interop import methods
  File "/root/.cache/bazel/_bazel_root/ec9ee79d770c6b7183036e592ac23980/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/pyt
hon/grpcio_tests/tests/interop/_secure_intraop_test.runfiles/com_github_grpc_grpc/src/python/grpcio_tests/tests/interop/methods.py", lin
e 24, in <module>
    from google.auth.transport import requests as google_auth_transport_requests
  File "/root/.cache/bazel/_bazel_root/ec9ee79d770c6b7183036e592ac23980/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/pyt
hon/grpcio_tests/tests/interop/_secure_intraop_test.runfiles/pypi__google_auth_1_5_1/google/auth/transport/requests.py", line 31, in <mo
dule>
    caught_exc,
  File "/root/.cache/bazel/_bazel_root/ec9ee79d770c6b7183036e592ac23980/execroot/com_github_grpc_grpc/bazel-out/k8-fastbuild/bin/src/pyt
hon/grpcio_tests/tests/interop/_secure_intraop_test.runfiles/pypi__six_1_11_0/six.py", line 737, in raise_from
    raise value
ImportError: The requests library is not installed, please install the requests package to use the requests transport.

SCENARIO 2:

Fresh instance with no PIP packages installed in current environment.
My requirements.txt:

...
google-auth>=1.0.0
requests>=2.14.2

My BUILD file:

...
py_library(
    name = "methods",
    srcs = ["methods.py"],
    deps = [
        requirement('google-auth'),
        requirement('requests'),
    ],
)
...

Observation

My Bazel test passes successfully.

Issues

This brings up 2 issues which are (probably) already reported:

• Related to whl: Parse requirements from METADATA files #90, rules_python doesn't read dependencies from METADATA files #70. Similar to Importing pip dependencies dependencies #110. rules_python does not work with packages that don't contain metadata.json files.
• The current pip rules can read system installed packages. But, well-known to be true.

The bigger issue is a question of inconsistency with specifying requirements. Does rules_python expect that all explicit and implicit dependencies be specified as a 'requirement'? In scenario 1, an implicit dependency was not declared but it was read nonetheless due to it being visible to Bazel. In scenario 2, it was needed to be specified a 'requirement' as well as explicitly added to 'requirements.txt' so that it could go ahead and install it.

If I didn't add it explicitly to 'requirements.txt', the error log was:

ERROR: /src/grpc/src/python/grpcio_tests/tests/interop/BUILD.bazel:27:1: Traceback (most recent call last):
        File "/src/grpc/src/python/grpcio_tests/tests/interop/BUILD.bazel", line 27
                py_library(name = "methods", srcs = ["methods..."], <2 more arguments>)
        File "/src/grpc/src/python/grpcio_tests/tests/interop/BUILD.bazel", line 36, in py_library
                requirement("requests")
        File "/root/.cache/bazel/_bazel_root/ec9ee79d770c6b7183036e592ac23980/external/grpc_python_dependencies/requirements.bzl", line 
138, in requirement
                fail(("Could not find pip-provided de...))
Could not find pip-provided dependency: 'requests'

This behaviour makes sense but makes it super easy to miss-out on specifying implicit dependencies for a package without realising it.

[josh-theorem]

At least in this case, it seems like the issue is that google-auth just has an undeclared dependency on requests:

https://github.com/GoogleCloudPlatform/google-auth-library-python/blob/master/setup.py#L21-L26

[nathanielmanistaatgoogle]

@theacodes can probably go into more detail about this (and can correct where I'm mistaken), but I think the case here is that requests is intended to be an "optional dependency", and that if you are a google-auth customer who intends to use the requests-using part of google-auth, then it's up to you to ensure that you have requests available in the environment in which you will be using the requests-using portion of google-auth.

@ghostwriternr: is "ensuring that we have requests available when knowing that we are going to be using the requests-using parts of google-auth" what this line does? (Because methods.py itself doesn't directly use requests.)

[theacodes]

You are correct, @nathanielmanistaatgoogle.

[ghostwriternr]

@nathanielmanistaatgoogle Thanks a ton! That does make a lot of sense. And yes, the line you've referred to in the Bazel BUILD file was added to address this expected dependency.

But I'd like to raise my question again. Does rules_python expect all explicit and implicit dependencies be specified as a 'requirement'? For instance here, requirement('requests') in deps for the respective rule? We'd probably still see the exact same behaviour even if requests was added explicitly as a dependency to google-auth.

[jo2y]

A partial answer is transitive package dependencies used to automatically work.

I haven't tracked down exactly what changed, but I think part of the problem is a metadata.json file isn't being created. Instead this code is failing back to the use the METADATA file. There is a TODO to handle fields other than name. So the BUILD file created is missing contents for deps.

Eg.

# Missing deps
$ ls bazel-workspace/external/pypi__requests_2_21_0/requests-2.21.0.dist-info/
LICENSE  METADATA  RECORD  top_level.txt  WHEEL
$ grep deps bazel-workspace/external/pypi__requests_2_21_0/BUILD
    deps = [],

# Working deps.
$ ls bazel-workspace/external/pypi__rsa_4_0/rsa-4.0.dist-info
DESCRIPTION.rst   LICENSE.txt  metadata.json  top_level.txt
entry_points.txt  METADATA     RECORD         WHEEL
$ grep deps bazel-workspace/external/pypi__rsa_4_0/BUILD
    deps = [requirement("pyasn1")],

[phb]

Was just hit by this issue because bazel 0.27 forced us to update a few packages. Is there any hope to see this issue fixed in the near term?

[joeleba]

+1 I'm seeing this as well

[satreix]

@ghostwriternr I think you are looking for #198. It does add support for packages that do not contain a metadata.json file. You may want to try to reproduce a transitive import with it.

I used the following:

load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")

git_repository(
    name = "rules_python",
    remote = "https://github.com/oapio/rules_python.git", # oapio:marian/whl_fix
    commit = "a158e6ad34fcc90a0a6cce663cd3153e7f8f86c3",  # https://github.com/bazelbuild/rules_python/pull/198
)

And successfully remove the following line from a py_binary of mine:

load("@rules_python//python:defs.bzl", "py_binary")
load("@pypi//:requirements.bzl", "requirement")

py_binary(
    name = "autoflake",
    srcs = ["entrypoint.py"],
    main = "entrypoint.py",
    deps = [
        requirement("autoflake"),
        # requirement("pyflakes"), Removed this transitive dep with success
    ],
)

[alexeagle]

rules_python 0.1.0 has been released which upstreams the rules_python_external repo. Please switch from pip_import to pip_install which doesn't have this issue.