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): Allow for Props classes to be passed into construct constructors #3346

Open
2 tasks
SamStephens opened this issue Sep 28, 2021 · 3 comments
Open
2 tasks
Labels
effort/medium Medium work item – a couple days of effort feature-request A feature should be added or improved. language/python Related to Python bindings p2

Comments

@SamStephens
Copy link

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.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@RomainMuller
Copy link
Contributor

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
Copy link
Contributor

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

@RomainMuller RomainMuller transferred this issue from aws/aws-cdk Jan 21, 2022
@RomainMuller RomainMuller added effort/medium Medium work item – a couple days of effort feature-request A feature should be added or improved. language/python Related to Python bindings p2 labels Jan 21, 2022
@polothy
Copy link
Contributor

polothy commented May 6, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/medium Medium work item – a couple days of effort feature-request A feature should be added or improved. language/python Related to Python bindings p2
Projects
None yet
Development

No branches or pull requests

3 participants