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

Python constructs do not implement their interfaces #13474

Closed
rectalogic opened this issue Mar 8, 2021 · 9 comments · Fixed by aws/jsii#2809
Closed

Python constructs do not implement their interfaces #13474

rectalogic opened this issue Mar 8, 2021 · 9 comments · Fixed by aws/jsii#2809
Assignees
Labels
bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@rectalogic
Copy link
Contributor

Many constructs in python do not implement their corresponding interfaces (e.g. aws_ec2.SecurityGroup does not implement aws_ec2.ISecurityGroup, Vpc does not implement IVpc, HostedZone does not implment IHostedZone etc.)

This causes typing errors since some methods are declared as accepting the interface and so will not accept the implementation.

Reproduction Steps

Run mypy on this:

#!/usr/bin/env python3

from aws_cdk import (
    core,
    aws_ec2 as ec2,
    aws_ecs as ecs,
    aws_route53 as route53,
    aws_ecs_patterns as ecs_patterns,
)


app = core.App()
stack = core.Stack(app, "stack")
vpc = ec2.Vpc(stack, "VPC", cidr="10.0.0.0/16")
sg = ec2.SecurityGroup(
    stack,
    "SG",
    vpc=vpc,
)
zone = route53.HostedZone(
    stack,
    "Zone",
    zone_name="example.com",
    vpcs=[vpc],
)
ecs_patterns.ApplicationLoadBalancedFargateService(
    stack,
    "FGS",
    vpc=vpc,
    security_groups=[sg],
    domain_name="f.example.com",
    domain_zone=zone,
    task_image_options=ecs_patterns.ApplicationLoadBalancedTaskImageOptions(
        image=ecs.ContainerImage.from_registry("foobar"),
        container_name="foobar",
    ),
)

app.synth()

What did you expect to happen?

No errors.

What actually happened?

Multiple typing errors.

$ mypy app.py
app.py:18: error: Argument "vpc" to "SecurityGroup" has incompatible type "Vpc"; expected "IVpc"
app.py:18: note: Following member(s) of "Vpc" have conflicts:
app.py:18: note:     Expected:
app.py:18: note:         def __jsii_proxy_class__() -> Type[_IVpcProxy]
app.py:18: note:     Got:
app.py:18: note:         def __jsii_proxy_class__() -> Type[_ResourceProxy]
app.py:24: error: List item 0 has incompatible type "Vpc"; expected "IVpc"
app.py:24: note: Following member(s) of "Vpc" have conflicts:
app.py:24: note:     Expected:
app.py:24: note:         def __jsii_proxy_class__() -> Type[_IVpcProxy]
app.py:24: note:     Got:
app.py:24: note:         def __jsii_proxy_class__() -> Type[_ResourceProxy]
app.py:29: error: Argument "vpc" to "ApplicationLoadBalancedFargateService" has incompatible type "Vpc"; expected "Optional[IVpc]"
app.py:30: error: List item 0 has incompatible type "SecurityGroup"; expected "ISecurityGroup"
app.py:30: note: Following member(s) of "SecurityGroup" have conflicts:
app.py:30: note:     Expected:
app.py:30: note:         def __jsii_proxy_class__() -> Type[_ISecurityGroupProxy]
app.py:30: note:     Got:
app.py:30: note:         def __jsii_proxy_class__() -> Type[_ResourceProxy]
app.py:32: error: Argument "domain_zone" to "ApplicationLoadBalancedFargateService" has incompatible type "HostedZone"; expected "Optional[IHostedZone]"
Found 5 errors in 1 file (checked 1 source file)

Environment

  • CDK CLI Version : 1.92.0 (build de536cc)
  • Framework Version: 1.92.0
  • Node.js Version: v14.15.0
  • OS : macOS 10.15.7
  • Language (Version): Python 3.8.7

Other


This is 🐛 Bug Report

@rectalogic rectalogic added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 8, 2021
@peterwoodworth
Copy link
Contributor

Haven't had time to try and reproduce yet - but this looks like it could be a versioning issue. Check your setup.py file to see if the aws-cdk packages are all the same version.

It's very possible something else is happening here, but let me know if that was the issue and if it's not I'll continue to investigate.

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 9, 2021
@rectalogic
Copy link
Contributor Author

Verified are all the same version. Looking at the python source you can see the issue, e.g. aws_cdk/aws_ec2/__init__.py has

@jsii.interface(jsii_type="@aws-cdk/aws-ec2.IVpc")
class IVpc(aws_cdk.core.IResource, typing_extensions.Protocol):
    @builtins.staticmethod
    def __jsii_proxy_class__() -> typing.Type["_IVpcProxy"]:

and

@jsii.implements(IVpc)
class Vpc(
    aws_cdk.core.Resource,
    metaclass=jsii.JSIIMeta,
    jsii_type="@aws-cdk/aws-ec2.Vpc",
):

So Vpc inherits core.Resource in aws_cdk/core/__init__.py which has:

@jsii.implements(IResource)
class Resource(
    Construct,
    metaclass=jsii.JSIIAbstractClass,
    jsii_type="@aws-cdk/core.Resource",
):
    '''A construct which represents an AWS resource.'''

    @builtins.staticmethod
    def __jsii_proxy_class__() -> typing.Type["_ResourceProxy"]:

So Vpc and IVpc have different return type signatures on __jsii_proxy_class__ and this is true for many classes and their interfaces in CDK, and so mypy complains.

@peterwoodworth
Copy link
Contributor

Thank you for the additional information. I'll have @MrArnoldPalmer take a look at this

@peterwoodworth peterwoodworth added needs-triage This issue or PR still needs to be triaged. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Mar 9, 2021
@gshpychka
Copy link
Contributor

@MrArnoldPalmer are there any updates regarding this? Do you need any help with reproducing it, or any other additional info?

@MrArnoldPalmer
Copy link
Contributor

@gshpychka no updates from us right now but this is in our backlog. Any help investigating and fixing is greatly appreciated of course if you're comfortable digging into the aws/jsii repository. You should be able to add a test using jsii-calc to cover this which is a start.

rectalogic added a commit to rectalogic/jsii that referenced this issue Apr 21, 2021
@rectalogic
Copy link
Contributor Author

@MrArnoldPalmer I added a test that reproduces this, the failure looks like:

tests/test_python.py::TestErrorHandling::test_implements_interface PASSED [100%]

=================================== FAILURES ===================================
_________________________________ test session _________________________________
mypy exited with status 1.
_____________________________ tests/test_python.py _____________________________
40: error: Argument 1 to "interface_func" has incompatible type "Vpc"; expected "IVpc"
40: note: 'Vpc' is missing following 'IVpc' protocol member:
40: note:     __jsii_proxy_class__
===================================== mypy =====================================
Found 1 error in 1 file (checked 20 source files)
=========================== short test summary info ============================
FAILED setup.py::mypy-status
FAILED tests/test_python.py::mypy
=================== 2 failed, 122 passed, 1 skipped in 6.53s ===================

/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/packages/@jsii/python-runtime/build-tools/_constants.ts:25
    throw new Error(
          ^
Error: Command failed with code 1: /Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/packages/@jsii/python-runtime/.env/bin/py.test -v --mypy
    at Object.runCommand (/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/packages/@jsii/python-runtime/build-tools/_constants.ts:25:11)
    at Object.<anonymous> (/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/packages/@jsii/python-runtime/build-tools/venv.ts:11:1)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Module.m._compile (/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/node_modules/ts-node/src/index.ts:1056:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Object.require.extensions.(anonymous function) [as .ts] (/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/node_modules/ts-node/src/index.ts:1059:12)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:831:12)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @jsii/python-runtime@0.0.0 test:run: `ts-node build-tools/venv.ts py.test -v --mypy`
npm ERR! Exit status 1

@rectalogic
Copy link
Contributor Author

Actually my repro is a slightly different error. jsii-calc is structured very differently than the CDK and I was having trouble replicating the CDK structure in jsii-calc. My testcase results in the base class not having __jsii_proxy_class__ instead of having a conflicting return value.

@rectalogic
Copy link
Contributor Author

OK, I pushed a better test that reproduces the original issue more closely

tests/test_python.py::TestErrorHandling::test_implements_interface PASSED [100%]

=================================== FAILURES ===================================
_________________________________ test session _________________________________
mypy exited with status 1.
_____________________________ tests/test_python.py _____________________________
40: error: Argument 1 to "interface_func" has incompatible type "Vpc"; expected "IVpc"
40: note: Following member(s) of "Vpc" have conflicts:
40: note:     Expected:
40: note:         def __jsii_proxy_class__() -> Type[_IVpcProxy]
40: note:     Got:
40: note:         def __jsii_proxy_class__() -> Type[_ResourceProxy]
===================================== mypy =====================================
Found 1 error in 1 file (checked 20 source files)
=========================== short test summary info ============================
FAILED setup.py::mypy-status
FAILED tests/test_python.py::mypy
=================== 2 failed, 122 passed, 1 skipped in 6.59s ===================

/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/packages/@jsii/python-runtime/build-tools/_constants.ts:25
    throw new Error(
          ^
Error: Command failed with code 1: /Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/packages/@jsii/python-runtime/.env/bin/py.test -v --mypy
    at Object.runCommand (/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/packages/@jsii/python-runtime/build-tools/_constants.ts:25:11)
    at Object.<anonymous> (/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/packages/@jsii/python-runtime/build-tools/venv.ts:11:1)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Module.m._compile (/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/node_modules/ts-node/src/index.ts:1056:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Object.require.extensions.(anonymous function) [as .ts] (/Users/aw/Projects/cureatr/dev/cureatr/experiments/jsii/node_modules/ts-node/src/index.ts:1059:12)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:831:12)

@MrArnoldPalmer MrArnoldPalmer added effort/medium Medium work item – several days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels May 17, 2021
mergify bot pushed a commit to aws/jsii that referenced this issue May 26, 2021

Fixes  aws/aws-cdk#13474

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants