pulumi / pulumi-confluentcloud

A Confluent Pulumi resource package, providing multi-language access to Confluent
Apache License 2.0
8 stars 3 forks source link

Remove resource_name_ requirement from Confluent.KafkaAcl class #366

Open tklusz opened 8 months ago

tklusz commented 8 months ago

Hello!

Issue details

When creating an instance of the KafkaAcl class, the property "resourcename" is required, where it shouldn't be, specifically when using Python.

Believe it is caused by this snippet: https://github.com/pulumi/pulumi-confluentcloud/blob/36f615d07e784de9cffcf3a139ae140f0101950c/sdk/python/pulumi_confluentcloud/kafka_acl.py#L480C1-L481C78

Affected area/feature

Confluent.KafkaAcl()

sdk/python/pulumi_confluentcloud/kafka_acl.py

iwahbe commented 7 months ago

Hi @tklusz. Can you explain why resource_name_ shouldn't be required?

tklusz commented 7 months ago

So you have a resource_name field for the name of the resource then a resource_name_ field that's also supposed to be the name. I am not sure why you need both of these.

I don't see resource_name_ being used anywhere in the code other than being a required class argument and used in a getter method when you pull a previously existing resource. If this is actually being used in code anywhere then sure it can be brought forward, but it should also be renamed.

I also don't know what your architectural decisions are but this is the only class in the provider that uses a separate required resource_name_ argument.

Your documentation references this as an optional variable, but it is actually required, see: