Skip to content
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

Transitive excludes on python requirements do not work for external libraries #11324

Open
jsirois opened this issue Dec 16, 2020 · 6 comments
Open

Comments

@jsirois
Copy link
Contributor

jsirois commented Dec 16, 2020

Transitive dependency excludes do not work for packages defined in external librarie's requirements.txt

Given this in 3rdparty/python/BUILD:

python_requirement_library(
   name="somelib",
   requirements=["somelib==1.0.0"]
)

python_requirement_library(
   name="someotherlib",
   requirements=["someotherlib==2.0.0"]
)

where somelib's requirements.txt includes someotherlib as a requirement, i.e.:
requirements.txt of somelib:

someotherlib==2.0.0

and this in src/mylib/BUILD

python_library(name="base")

python_awslambda(
    name="awslambda",
    dependencies=[
        ":base",
        "3rdparty/python:somelib",
        "!!3rdparty/python:someotherlib",
    ],
    runtime="python3.8",
    handler="lambda_function:lambda_handler",
)

and running ./pants package src/mylib:awslambda, the resulting package still includes someotherlib (most likely because somelib needs it in its own requirements.txt, and pants does not look like it's aware of that.).

For my use case, I'm trying to use an external library which makes use of boto3 and botocore: aws-secretsmanager-caching, and I need the lambda package to not contain the boto3and botocorepackages, because they take too much space in the lambda package and the lambda runtime already includes them out of the box.

The current workaround I have is to clone the aws-secretsmanager-cachinglibrary, convert it to a pants accepted build folder using BUILD files, and then the exclude works as expected, however this is not ideal since it copies source code from another library into my project.

Any better way to do this ? Is this a feature that's supposed to be supported by pants ? Thanks for your feedback.

@jsirois
Copy link
Contributor Author

jsirois commented Dec 16, 2020

Moved this issue here from pex-tool/pex#1146 for @GLeurquin.

@GLeurquin
Copy link

Thanks @jsirois, indeed had the wrong tab opened (pex instead of pants).

I'll add that the version of pants I'm using is: 2.1.0

@Eric-Arellano
Copy link
Contributor

Hm, John, I think this is actually a Pex issue.

Is this a feature that's supposed to be supported by pants ?

Not in the current form, no. The current transitive excludes code deals in the abstraction level of targets. You can say to ignore a python_requirement_library, which removes it from the inputs to the command pex -o app.pex req1 req2 req3.

But what another python_requirement_library's dependencies are—as expressed in its setup.py—is completely out of the domain of Pants, and should be. Pants has no notion of what a top-level requirement's transitive deps are.

To support this, Pants would need to do something like pex -o app.pex req1 req2 req3 --exclude req4. @jsirois, initial thoughts on Pex having an --exclude feature?

@jsirois
Copy link
Contributor Author

jsirois commented Dec 17, 2020

Hm, John, I think this is actually a Pex issue.

I don't think so. The python community does not support requirement excludes in any tool I'm aware of or any relevant PEP. If Pants wants to extend its exclude mechanism for local code to remote code, it will need to work around ecosystems that do not support this for 3rdparty deps. So, for the jvm it can leverage maven excludes, for Python it cannot.

Some relevant issues in the python community asking for this feature from dependency resolvers:
pypa/pip#3090 (comment)
python-poetry/poetry#697 (comment)

To support this, Pants would need to do something like pex -o app.pex req1 req2 req3 --exclude req4. @jsirois, initial thoughts on Pex having an --exclude feature?

Pex should definitely not support this.

@Eric-Arellano
Copy link
Contributor

Hm, John, I think this is actually a Pex issue.

I don't think so.

+1, this then is really a pip issue.

Pex should definitely not support this.

Likewise, I don't think Pants should support this without Pex supporting it, which requires pip / Python ecosystem supporting it.

I'm going to close as out of scope @GLeurquin.

@cburroughs
Copy link
Contributor

Reopening due to Pex support.

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

No branches or pull requests

4 participants