jsii icon indicating copy to clipboard operation
jsii copied to clipboard

MyPy throwing type errors when it shouldn't

Open alexmv opened this issue 5 years ago • 15 comments

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: https://github.com/aws/jsii/issues/1262


This is :bug: Bug Report

alexmv avatar Apr 02 '20 16:04 alexmv

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 avatar Apr 06 '20 07:04 rix0rrr

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 avatar Apr 06 '20 16:04 alexmv

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

alexmv avatar Apr 10 '20 01:04 alexmv

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 avatar Apr 10 '20 02:04 skinny85

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__.

alexmv avatar Apr 10 '20 04:04 alexmv

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 avatar Apr 10 '20 09:04 rix0rrr

Paging the jsii team :)

rix0rrr avatar Apr 10 '20 09:04 rix0rrr

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.

MrArnoldPalmer avatar Apr 10 '20 20:04 MrArnoldPalmer

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"

salimhamed avatar Jun 24 '20 01:06 salimhamed

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

Ranjith072 avatar Jan 29 '21 07:01 Ranjith072

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

alexmv avatar Jan 29 '21 07:01 alexmv

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

Ranjith072 avatar Jan 29 '21 08:01 Ranjith072

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

Ranjith072 avatar Jan 29 '21 08:01 Ranjith072

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

gshpychka avatar Apr 14 '21 09:04 gshpychka

@MrArnoldPalmer @RomainMuller Running into this error as well.

Arvanaghi avatar Apr 20 '21 19:04 Arvanaghi

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:

  • https://github.com/aws/jsii/pull/2809
  • https://github.com/aws/jsii/pull/4611

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 avatar Sep 03 '24 14:09 iliapolo

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.

github-actions[bot] avatar Sep 03 '24 14:09 github-actions[bot]

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.

github-actions[bot] avatar Sep 09 '24 07:09 github-actions[bot]