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

MyPy throwing type errors when it shouldn't #3931

Closed
alexmv opened this issue Apr 2, 2020 · 18 comments
Closed

MyPy throwing type errors when it shouldn't #3931

alexmv opened this issue Apr 2, 2020 · 18 comments
Labels
bug This issue is a bug. effort/large Large work item – several weeks of effort language/python Related to Python bindings p1

Comments

@alexmv
Copy link

alexmv commented Apr 2, 2020

In the Python CDK, attempting to pass aws_events_targets.LambdaFunction as the target of an events.Rule fails to typecheck:

$ mypy test.py
test.py:19: error: List item 0 has incompatible type "LambdaFunction"; expected "IRuleTarget"
test.py:19: note: 'LambdaFunction' is missing following 'IRuleTarget' protocol member:
test.py:19: note:     __jsii_proxy_class__
Found 1 error in 1 file (checked 1 source file)

It does run perfectly fine, however -- this is purely an error in the typesystem.

Reproduction Steps

from aws_cdk import aws_events, aws_events_targets, aws_lambda, core

class Example(core.Construct):
    def __init__(self, scope: core.Construct):
        super().__init__(scope, "some-name")
        some_lambda = aws_lambda.Function(
            self,
            "some-handler",
            runtime=aws_lambda.Runtime.PYTHON_3_8,
            code=aws_lambda.AssetCode("some/path"),
            handler="some_package.handler",
            retry_attempts=0,
        )
        aws_events.Rule(
            self,
            "some-rule",
            schedule=aws_events.Schedule.cron(minute="*/5"),
            targets=[aws_events_targets.LambdaFunction(some_lambda)],
        )

pip install mypy && mypy test.py

Error Log

test.py:19: note: 'LambdaFunction' is missing following 'IRuleTarget' protocol member:
test.py:19: note:     __jsii_proxy_class__

Environment

  • CLI Version: 1.17.13
  • Framework Version: 1.31.0
  • OS: OS X 10.14.6
  • Language: Python 3.8.0 / mypy 0.770

Other

It is also possible this is a bug in mypy.


Possible duplicate: #1262


This is 🐛 Bug Report

@alexmv alexmv added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 2, 2020
@SomayaB SomayaB added the language/python Related to Python bindings label Apr 3, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 6, 2020

You are supposed to use this integration class to glue them together: https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-events-targets.LambdaFunction.html

We use this pattern in many places.

@rix0rrr rix0rrr closed this as completed Apr 6, 2020
@rix0rrr rix0rrr added guidance Question that needs advice or information. and removed bug This issue is a bug. labels Apr 6, 2020
@alexmv
Copy link
Author

alexmv commented Apr 6, 2020

Please re-read the code I posted. I am using the aws_cdk.aws_events_targets.LambdaFunction class. The bug is that LambdaFunction does not implement IRuleTarget as it is documented to, because it's missing a __jsii_proxy_class__ method.

Which appears to be accurate. Looking at this more closely, IRuleTarget is a protocol which defines the non-abstract static method __jsii_proxy_class__, as well as bind. Because both are part of the protocol's definition, it means that LambdaFunction must itself define a static __jsii_proxy_class__ method to be an IRuleTarget, which it doesn't currently:

$ python
Python 3.8.0 (default, Mar 18 2020, 12:21:22)
[Clang 10.0.1 (clang-1001.0.46.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from aws_cdk import aws_events, aws_events_targets
>>> aws_events_targets.LambdaFunction.__jsii_proxy_class__()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object 'LambdaFunction' has no attribute '__jsii_proxy_class__'

@alexmv
Copy link
Author

alexmv commented Apr 10, 2020

Heya -- I believe this bug was closed in error; see my post from a few days ago. Can it be re-opened?

@skinny85
Copy link
Contributor

skinny85 commented Apr 10, 2020

Sorry about that @alexmv !

I just tried with the following code:

        some_lambda = aws_lambda.Function(
            self, "some-handler",
            runtime=aws_lambda.Runtime.PYTHON_3_8,
            code=aws_lambda.CfnParametersCode(),
            handler="some_package.handler",
            retry_attempts=0,
        )
        aws_events.Rule(
            self, "some-rule",
            schedule=aws_events.Schedule.cron(minute="*/5"),
            targets=[aws_events_targets.LambdaFunction(some_lambda)],
        )

and I got the following template:

Resources:
  ServiceCatalogPipelinesomehandlerServiceRoleEC8BC838:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: lambda.amazonaws.com
        Version: "2012-10-17"
      ManagedPolicyArns:
        - Fn::Join:
            - ""
            - - "arn:"
              - Ref: AWS::Partition
              - :iam::aws:policy/service-role/AWSLambdaBasicExecutionRole
    Metadata:
      aws:cdk:path: hello-cdk-1/ServiceCatalogPipeline/some-handler/ServiceRole/Resource
  ServiceCatalogPipelinesomehandler9B981751:
    Type: AWS::Lambda::Function
    Properties:
      Code:
        S3Bucket:
          Ref: ServiceCatalogPipelinesomehandlerLambdaSourceBucketNameParameter56C9663D
        S3Key:
          Ref: ServiceCatalogPipelinesomehandlerLambdaSourceObjectKeyParameter5B5EE148
      Handler: some_package.handler
      Role:
        Fn::GetAtt:
          - ServiceCatalogPipelinesomehandlerServiceRoleEC8BC838
          - Arn
      Runtime: python3.8
    DependsOn:
      - ServiceCatalogPipelinesomehandlerServiceRoleEC8BC838
    Metadata:
      aws:cdk:path: hello-cdk-1/ServiceCatalogPipeline/some-handler/Resource
  ServiceCatalogPipelinesomehandlerEventInvokeConfig36490DD4:
    Type: AWS::Lambda::EventInvokeConfig
    Properties:
      FunctionName:
        Ref: ServiceCatalogPipelinesomehandler9B981751
      Qualifier: $LATEST
      MaximumRetryAttempts: 0
    Metadata:
      aws:cdk:path: hello-cdk-1/ServiceCatalogPipeline/some-handler/EventInvokeConfig/Resource
  ServiceCatalogPipelinesomehandlerAllowEventRulehellocdk1ServiceCatalogPipelinesomerule0752F509CACFF320:
    Type: AWS::Lambda::Permission
    Properties:
      Action: lambda:InvokeFunction
      FunctionName:
        Fn::GetAtt:
          - ServiceCatalogPipelinesomehandler9B981751
          - Arn
      Principal: events.amazonaws.com
      SourceArn:
        Fn::GetAtt:
          - ServiceCatalogPipelinesomeruleE4CCF127
          - Arn
    Metadata:
      aws:cdk:path: hello-cdk-1/ServiceCatalogPipeline/some-handler/AllowEventRulehellocdk1ServiceCatalogPipelinesomerule0752F509
  ServiceCatalogPipelinesomeruleE4CCF127:
    Type: AWS::Events::Rule
    Properties:
      ScheduleExpression: cron(*/5 * * * ? *)
      State: ENABLED
      Targets:
        - Arn:
            Fn::GetAtt:
              - ServiceCatalogPipelinesomehandler9B981751
              - Arn
          Id: Target0
    Metadata:
      aws:cdk:path: hello-cdk-1/ServiceCatalogPipeline/some-rule/Resource
Parameters:
  ServiceCatalogPipelinesomehandlerLambdaSourceBucketNameParameter56C9663D:
    Type: String
  ServiceCatalogPipelinesomehandlerLambdaSourceObjectKeyParameter5B5EE148:
    Type: String

So cdk synth works, however I do get a warning in my IDE that aws_events_targets.LambdaFunction does not implement IRuleTarget - so there is definitely something to be improved here!

@skinny85 skinny85 reopened this Apr 10, 2020
@alexmv
Copy link
Author

alexmv commented Apr 10, 2020

Sorry for not making clear that yeah, this is not about the functionality (which works great!), but about the type-correctness of the jsii-generated typing of the code.

I've found two other ones: neither aws_cloudwatch.MathExpression nor aws_cloudwatch.Metric implement IMetric as they are documented to, also because they don't implement __jsii_proxy_class__.

@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 10, 2020

Sorry about closing your issue before, I must have misread.

Yes, this would seem to be an issue in the Python bindings generated by jsii then.

But in that case, shouldn't all interfaces have this problem?

@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 10, 2020

Paging the jsii team :)

@rix0rrr rix0rrr added bug This issue is a bug. and removed guidance Question that needs advice or information. labels Apr 10, 2020
@MrArnoldPalmer
Copy link
Contributor

We recently had MyPy start throwing some new type errors that previously it wasn't. Its likely that previously it wasn't catching this missing implementation and this was fixed in a recent release of MyPy. We can investigate this on the JSII side but its likely that the python bindings are missing MyPy annotations in places. We will have to figure out why this isn't caught during build time as well.

@rix0rrr rix0rrr removed their assignment Apr 14, 2020
@RomainMuller RomainMuller added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 14, 2020
@salimhamed
Copy link

salimhamed commented Jun 24, 2020

I'm running into the same issue with aws_cloudwatch.Metric not correctly implementing IMetric. Here's a snippet that demonstrates the issue:

import aws_cdk.aws_cloudwatch as cloudwatch
import aws_cdk.aws_kinesis as kds
from aws_cdk import core


class MyStack(core.Stack):
    def __init__(self, scope: core.Construct, _id: str, **kwargs) -> None:
        super().__init__(scope, _id, **kwargs)

        self.template_options.description = "Demo"

        stream = kds.Stream(self, "InputStream", shard_count=16)

        dashboard = cloudwatch.Dashboard(self, "Dashboard", dashboard_name=core.Aws.STACK_NAME)

        incoming_records = cloudwatch.Metric(
            namespace="AWS/Kinesis",
            metric_name="IncomingRecords",
            dimensions={"StreamName": stream.stream_name},
            period=core.Duration.minutes(1),
            statistic="sum",
        )

        dashboard.add_widgets(
            cloudwatch.GraphWidget(
                left=[incoming_records],  # 'Metric' is missing following 'IMetric' protocol member
                width=24,
                title="Kinesis data stream (incoming)",
                left_y_axis=cloudwatch.YAxisProps(min=0),
                right_y_axis=cloudwatch.YAxisProps(min=0),
            )
        )

Checking types with Mypy results in the following error:

'Metric' is missing following 'IMetric' protocol member:
__jsii_proxy_class__
List item 0 has incompatible type "Metric"; expected "IMetric"

@Ranjith072
Copy link

what is the workaround for this issue ? i suddenly started to get this error in cdk 1.83.0

@alexmv
Copy link
Author

alexmv commented Jan 29, 2021

# type: ignore applied judiciously should work around this bug.

@Ranjith072
Copy link

my project is too big and its complaining more than 1000 times :( any other alternative than supressing the type checking on the package?

@Ranjith072
Copy link

suddenly it is complaining on all the interfaces i use in cdk.
ex:
image

@gshpychka
Copy link

@MrArnoldPalmer @RomainMuller any updates on this? All interfaces have this error for me.

@Arvanaghi
Copy link

@MrArnoldPalmer @RomainMuller Running into this error as well.

@RomainMuller RomainMuller removed their assignment Jun 21, 2021
@MrArnoldPalmer MrArnoldPalmer removed their assignment Jun 21, 2021
@madeline-k madeline-k changed the title LambdaFunction does not implement IRuleTarget MyPy throwing type errors when it shouldn't Jan 26, 2023
@madeline-k madeline-k transferred this issue from aws/aws-cdk Jan 26, 2023
@mrgrain mrgrain added p1 and removed p2 labels Apr 18, 2024
@iliapolo
Copy link
Contributor

iliapolo commented Sep 3, 2024

It has been a long while since this issue was last commented on. Both MyPy, CDK, and jsii have gone through many upgrades. jsii in particular had a few __jsii_proxy_class__ related PRs:

Closing as I am unable to reproduce this as of now. Please open a new issue with an up do date reproduction if you encounter this issue again.

@iliapolo iliapolo closed this as completed Sep 3, 2024
Copy link
Contributor

github-actions bot commented Sep 3, 2024

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

@iliapolo iliapolo closed this as not planned Won't fix, can't repro, duplicate, stale Sep 9, 2024
Copy link
Contributor

github-actions bot commented Sep 9, 2024

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

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/large Large work item – several weeks of effort language/python Related to Python bindings p1
Projects
None yet
Development

No branches or pull requests