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

Issues with AWS CDK >= 2.51.0 #914

Closed
psalvaggio-dl opened this issue Dec 4, 2022 · 12 comments
Closed

Issues with AWS CDK >= 2.51.0 #914

psalvaggio-dl opened this issue Dec 4, 2022 · 12 comments
Labels
Can Close? Will close in 30 days if there is no new activity

Comments

@psalvaggio-dl
Copy link

🐞 bug report

Affected Rule

requirement
pip_parse

Is this a regression?

No

Description

With AWS CDK 2.51.0, they have split out a number of sub-packages. Not sure why as they are required dependencies, but nonetheless, if you depend on aws-cdk-lib, you also now depend on:

aws-cdk.asset-awscli-v1
aws-cdk.asset-kubectl-v20
aws-cdk.asset-node-proxy-agent-v5

This seems to break tests in my monorepo. I am not sure exactly why, but my theory is that there are now multiple aws-cdk/__init__.py's in my PYTHONPATH due to rules_python's PYTHONPATH-based solution for handling pip dependencies.

🔬 Minimal Reproduction

I have made a minimal example here: https://github.com/psalvaggio-dl/bazel-cdk-repro

In order to reproduce, the bazel test //py:test can be run.

🔥 Exception or Error

My pytest test prints out the following error:


ImportError while importing test module '/home/ubuntu/.cache/bazel/_bazel_ubuntu/f5bb56bb935f39261247d94ebd292fdf/sandbox/linux-sandbox/5/execroot/bazel-cdk-repro/bazel-out/k8-fastbuild/bin/py/test.runfiles/bazel-cdk-repro/py/test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
py/test.py:1: in 
    from aws_cdk import Aws
E   ImportError: cannot import name 'Aws' from 'aws_cdk' (/home/ubuntu/.cache/bazel/_bazel_ubuntu/f5bb56bb935f39261247d94ebd292fdf/sandbox/linux-sandbox/5/execroot/bazel-cdk-repro/bazel-out/k8-fastbuild/bin/py/test.runfiles/pip_aws_cdk_asset_awscli_v1/site-packages/aws_cdk/__init__.py)
=========================== short test summary I

Aws we can see in the error, it seems to thing that the aws_cdk/__init__.py in pip_aws_cdk_asset_awscli_v1 is the right one, which is why I feel like having multiples in the PYTHONPATH is the issue.

🌍 Your Environment

Operating System:

Ubuntu 22.04

Output of bazel version:

  
bazel 5.3.2
  

Rules_python version:

  
0.15.1
  

Anything else relevant?

@philsc
Copy link
Contributor

philsc commented Dec 5, 2022

You can fix that particular error you posted by doing this:

diff --git a/WORKSPACE.bazel b/WORKSPACE.bazel
index b3c4386..4a986e1 100644
--- a/WORKSPACE.bazel
+++ b/WORKSPACE.bazel
@@ -43,6 +43,7 @@ pip_parse(
     name = "pip",
     requirements_lock = "//py:requirements.txt.lock",
     python_interpreter_target = py_interpreter,
+    enable_implicit_namespace_pkgs = True,
 )

 load("@pip//:requirements.bzl", lazy_pip_install = "install_deps")

That, together with --incompatible_default_to_explicit_init_py gets you past that error.

Unfortunately, you then run into this error:

ImportError while importing test module '/home/phil/.cache/bazel/_bazel_phil/03812bb59b76e0d44a0c22cc9583226f/sandbox/linux-sandbox/17/execroot/bazel-cdk-repro/bazel-out/k8-fastbuild/bin/py/test.runfiles/bazel-cdk-repro/py/test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
py/test.py:1: in <module>
    from aws_cdk import Aws
../pip_aws_cdk_lib/site-packages/aws_cdk/__init__.py:1257: in <module>
    from ._jsii import *
../pip_aws_cdk_lib/site-packages/aws_cdk/_jsii/__init__.py:13: in <module>
    import aws_cdk.asset_awscli_v1._jsii
E   ModuleNotFoundError: No module named 'aws_cdk.asset_awscli_v1'

Digging into it, it seems to be happening because the aws-cdk.lib package contains an __init__.py file in that namespace:

bazel-out/k8-fastbuild/bin/py/test.runfiles/bazel-cdk-repro/external/pip_aws_cdk_lib/site-packages/aws_cdk/__init__.py

I.e. it never finds the following file(s) because Python stopped looking for namespace packages named aws_cdk:

bazel-out/k8-fastbuild/bin/py/test.runfiles/bazel-cdk-repro/external/pip_aws_cdk_asset_awscli_v1/site-packages/aws_cdk/asset_awscli_v1

That means aws-cdk.lib doesn't support namespace packages even though it's distributing its code as such. I would be tempted to call this a bug with aws-cdk.lib, not with rules_python.

psalvaggio-dl added a commit to psalvaggio-dl/bazel-cdk-repro that referenced this issue Dec 5, 2022
@psalvaggio-dl
Copy link
Author

Thanks for looking into it. Yes, I can verify that behavior and have updated the reproduction case. I also have an issue open in the CDK aws/jsii#3881, so I will post this over there as well to see if there is something they can fix on their end.

@psalvaggio-dl
Copy link
Author

I got some feedback on the Bazel slack and was able to push a fix up to the aspect_rules branch of the repo. It required using the aspect_rules_py for depending on wheels of the AWS CDK directly. This worked in both the small reproduction case as well as my bigger monorepo. This solution is fine with me, but do we want to leave this issue open to see if there might be a way to fix this solely with rules_python?

@psalvaggio-dl
Copy link
Author

@philsc I reposted you comment over on the bug in the CDK and they are asking for a bit of clarification on why you think their library doesn't support namespace packages? Could you elaborate on this a bit so I can get direct them on what might need to change in their library to make this work without too much workaround?

@philsc
Copy link
Contributor

philsc commented Dec 6, 2022

I replied there directly 👍

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Jun 11, 2023
@psalvaggio-dl
Copy link
Author

Commenting since the issue got marked stale. This is still blocking me upgrading this AWS dependency, however, it sounds like this is a bug on their side and a fix on rules_python's side would introduce non-standard compliant behavior that may lead to other issues.

@github-actions github-actions bot removed the Can Close? Will close in 30 days if there is no new activity label Jun 12, 2023
@aignas
Copy link
Collaborator

aignas commented Nov 5, 2023

With #1393 you should be able to apply patches to wheels, although it only supports bzlmod. Would that be something that is good enough for your use case?

@aignas
Copy link
Collaborator

aignas commented Dec 15, 2023

FYI, for legacy WORKSPACE users it is technically possible to pass whl_patches argument to install_deps() call in your WORKSPACE. The API is not something we want to make a promise to maintain backwards compatibility for, hence the documentation that it is internal use only. As we evolve the APIs to better support patching and cross-platform dependency resolution, this may change, so please be warned, but at least this can give a stop-gap solution without requiring users to patch rules_python itself.

The source code documenting on what you would need to pass is https://github.com/bazelbuild/rules_python/blob/main/python/pip_install/pip_repository.bzl#L847.

@AndrewGuenther
Copy link

@aignas A wheel patch won't fix this because the __init__.py at the top level of the aws_cdk_lib package has content in it. It is not a pure namespace package.

Copy link

This issue has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Jun 19, 2024
Copy link

This issue was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? Will close in 30 days if there is no new activity
Projects
None yet
Development

No branches or pull requests

4 participants