aws / jsii

jsii allows code in any language to naturally interact with JavaScript classes. It is the technology that enables the AWS Cloud Development Kit to deliver polyglot libraries from a single codebase!
https://aws.github.io/jsii
Apache License 2.0
2.63k stars 244 forks source link

(python): Allow for Props classes to be passed into construct constructors #3346

Open SamStephens opened 2 years ago

SamStephens commented 2 years ago

In other languages, the properties for an L2 construct are encapsulated in a Props class, and passed into the constructor of that class.

For example, to create a lambda Function you pass in an instance of FunctionProps.

However for Python, the constructor for Function takes all the properties as named parameters. In the generated source code, an instance of FunctionProps is constructed from those named parameters. However it is not possible to use an instance of FunctionProps to construct a Function.

Allowing for props to be provided either using the current method, or by providing an instance of the Props class would unlock some use cases.

Use Case

My use case is that I'm trying to build a library of standard constructs for my team's use. In particular, I am creating standard Cloudwatch definitions for use across our applications.

I want our alarms to be generated based on the attributes of the constructs being monitored. For example, for Lambda's, I want an alarm that fires if execution durations are reaching 90% of the handler timeout. For Elasticsearch domains, I want to know the EBS volume size so I can know when disk space is less than 5% of the volume size.

The actual CDK constructs themselves, aws_lambda.Function, aws_elasticsearch.Domain and so on, are not suitable for this because they do not expose all their properties to be publicly read.

The Props classes would be ideal for this. I could configure my CDK construct with an instance of a Props class, and then also pass that into my custom constructs.

I could use this solution today with another language; this limitation only exists in Python.

Proposed Solution

Because the Python classes are generated, it shouldn't be too difficult to add an overload in a backwards compatible fashion. Simply generate an additional keyword argument called props for every CDK L2 construct, and allow construction either using the existing keyword argument, or by providing a Props class to props. I see usage looking like:

        lambda_function_props = aws_lambda.FunctionProps(
            handler="index.handler",
            runtime=aws_lambda.Runtime.PYTHON_3_9,
            code=aws_lambda.Code.asset("lambdas/module"),
        )
        lambda_function = aws_lambda.Function(
            scope=self,
            id="ElasticsearchIndexManagerFunction",
            props=lambda_function_props,
        )

Which would then allow my reuse of the properties elsewhere.

        lambda_alarming = CustomLambdaAlarmingConstruct(
            scope=self,
            id="CustomLambdaAlarmingConstruct",
            lambda_function_props = lambda_function_props,
            alarm_topic= alarm_topic,
        )

Other

The one potential issue I see here is that there'd need to be no conflicts with the props keyword within the Props classes; hopefully that's a reserved word as scope and id clearly must be.


This is a :rocket: Feature Request

RomainMuller commented 2 years ago

The one potential issue I see here is that there'd need to be no conflicts with the props keyword within the Props classes; hopefully that's a reserved word as scope and id clearly must be.

In fact, scope and id are conventions, not reserved words.

jsii has this process where if a keyword argument conflicts with a positional argument, we'll append _ at the end of the positional argument's name until it no longer clashes. This in turn might become a problem as Python allows all arguments to be used by-name (and hence, changing the name breaks those call sites).

We might be able to use @overload to define one signature that accepts the **kwargs bundle, and one signature that accepts the props value? I haven't thought about it enough to know whether this would be workable or not...

Anyhow -- I think adding a new props keyword to accept the properties object feels like it might cause more confusion than it helps... In addition to the name collision risk. I wouldn't resort to that option unless it's the only way this could work (and we decide it's worth the risks).


Ref: https://mypy.readthedocs.io/en/stable/more_types.html#function-overloading

RomainMuller commented 2 years ago

Also - this feature request belongs with jsii so I'm moving it to the aws/jsii repository.

polothy commented 4 months ago

This would be very helpful. We internally implemented the idea of secure properties from this RFC. I realized this too late because I hadn't tried it in Python until adoption was already pretty high internally. The usage for Python isn't ideal:

compliance_queue_props: QueueProps = Sqs.queue(self)
queue = Queue(self, "Resource", **compliance_queue_props._values)

It's been a while, but IIRC, you have to use ._values otherwise it'll pass in other fields that aren't properties of Queue. This all works fine though in TypeScript since properties are just interfaces/objects.