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: issues when implementing an interface #577

Closed
eladb opened this issue Jul 2, 2019 · 1 comment · Fixed by #586
Closed

python: issues when implementing an interface #577

eladb opened this issue Jul 2, 2019 · 1 comment · Fixed by #586
Labels
language/python Related to Python bindings

Comments

@eladb
Copy link
Contributor

eladb commented Jul 2, 2019

This example:

from aws_cdk.core import Stack, Construct, IAspect, IConstruct
import jsii

@jsii.implements(IAspect)
class MonitoringAspect:
    def visit(self, node: IConstruct) -> None:
        print("visit %s" % node)

class MyStack(Stack):
    def __init__(self, scope: Construct, cid: str, **kwarg) -> None:
        super().__init__(scope, cid, **kwarg)

        self.node.apply_aspect(MonitoringAspect()) # <-- WARNING: Expected type IAspect got MonitoringStack

__all__ = ["MyStack"]

Has the following issues:

  1. Warning that says that apply_aspect expects IAspect and got MonitoringStack.
  2. When visit is called, node is an ObjRef object instead of an IConstruct.

This is the output:

visit ObjRef(ref='@aws-cdk/core.Stack@10001')
visit ObjRef(ref='@aws-cdk/aws-dynamodb.Table@10004')
visit ObjRef(ref='@aws-cdk/aws-dynamodb.CfnTable@10019')
visit ObjRef(ref='@aws-cdk/core.Resource@10020')
visit ObjRef(ref='@aws-cdk/aws-lambda.Function@10008')
visit ObjRef(ref='@aws-cdk/aws-iam.Role@10021')
visit ObjRef(ref='@aws-cdk/aws-iam.CfnRole@10022')
visit ObjRef(ref='@aws-cdk/aws-iam.Policy@10023')
visit ObjRef(ref='@aws-cdk/aws-iam.CfnPolicy@10024')
visit ObjRef(ref='@aws-cdk/aws-lambda.CfnFunction@10025')
visit ObjRef(ref='@aws-cdk/aws-s3-assets.Asset@10026')
visit ObjRef(ref='@aws-cdk/assets.Staging@10027')
visit ObjRef(ref='@aws-cdk/core.CfnParameter@10028')
visit ObjRef(ref='@aws-cdk/core.CfnParameter@10029')
visit ObjRef(ref='@aws-cdk/core.CfnParameter@10030')
visit ObjRef(ref='@aws-cdk/core.Resource@10031')
visit ObjRef(ref='@aws-cdk/aws-lambda.CfnPermission@10032')
visit ObjRef(ref='@aws-cdk/aws-lambda.CfnPermission@10033')

Looks like the objref is not deserialized in the callback.

@eladb
Copy link
Contributor Author

eladb commented Jul 2, 2019

Another example of a jsii-generated class that implements an interface but fails type check:

aws_route53.ARecord(
    self, 'UrlShortenerDomain',
    record_name='go',
    zone=hosted_zone,
    target=aws_route53.RecordTarget.from_alias(aws_route53_targets.ApiGateway(api)))

Error is: expected type IAliasRecordType, got ApiGateway instead.

@fulghum fulghum added the language/python Related to Python bindings label Jul 2, 2019
@eladb eladb closed this as completed in #586 Jul 7, 2019
eladb pushed a commit that referenced this issue Jul 7, 2019
This change addresses 4 issues:

- Structs now use idiomatic (snake_case) capitalization of fields
  (instead of JSII-inherited camelCase).
- IDE support -- replace TypedDict usage with regular classes. This
  makes it so that we can't use dicts anymore, but mypy support in
  IDEs wasn't great and by using POPOs (Plain Old Python Objects)
  IDEs get their support back.
- Structs in a variadic argument use to be incorrectly lifted to
  keyword arguments, this no longer happens.
- Stop emitting "Stable" stabilities in docstrings, "Stable" is implied.

In order to make this change, I've had to make `jsii-pacmak` depend on
`jsii-reflect`. This is the proper layering of libraries anyway, since
`jsii-reflect` adds much-needed--and otherwise
duplicated--interpretation of the spec.

Complete refactoring of `jsii-pacmak` to build on `jsii-reflect` is too
much work right now, however, so I've added "escape hatching" where
generators can take advantage of the power of jsii-reflect if they want
to, but most of the code still works on the raw spec level.

Added a refactoring where we load the assembly once and reuse the same
instance for all generators, instead of loading the assembly for every
generator. Assembly-loading, especially with a lot of dependencies, takes
a non-negligible amount of time, so this has the side effect of making
the packaging step faster (shaves off 100 packages * 3 targets * a
couple of seconds).

Fixes #537 
Fixes #577 
Fixes #578 
Fixes #588
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language/python Related to Python bindings
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants