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

Cannot call a method that takes an empty struct from Python #2846

Closed
1 of 5 tasks
skinny85 opened this issue May 19, 2021 · 4 comments · Fixed by #3372
Closed
1 of 5 tasks

Cannot call a method that takes an empty struct from Python #2846

skinny85 opened this issue May 19, 2021 · 4 comments · Fixed by #3372
Labels
bug This issue is a bug. cdk-blocker language/python Related to Python bindings p1

Comments

@skinny85
Copy link
Contributor

skinny85 commented May 19, 2021

🐛 Bug Report

Affected Languages

  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...)
  • Go

General Information

  • JSII Version: 1.29.0
  • Platform: Darwin 186590cdb71d.ant.amazon.com 18.7.0 Darwin Kernel Version 18.7.0: Mon Mar 8 22:11:48 PST 2021; root:xnu-4903.278.65~1/RELEASE_X86_64 x86_64

What is the problem?

We have the following interface set in CDK's RDS library:

export interface IParameterGroup extends IResource {
  bindToCluster(options: ParameterGroupClusterBindOptions): ParameterGroupClusterConfig;

  bindToInstance(options: ParameterGroupInstanceBindOptions): ParameterGroupInstanceConfig;

  // doesn't matter...
}

export interface ParameterGroupClusterBindOptions {
}

export interface ParameterGroupClusterConfig {
  readonly parameterGroupName: string;
}

export interface ParameterGroupInstanceBindOptions {
}

export interface ParameterGroupInstanceConfig {
  readonly parameterGroupName: string;
}

export class ParameterGroup extends Resource implements IParameterGroup {
  // ...

Turns out, you can't call the bindToCluster() and bindToInstance() methods at all from Python.

Assuming we have this:

        parameter_group = rds.ParameterGroup(
            self, 'ParameterGroup',
            engine=rds.DatabaseInstanceEngine.maria_db(
                version=rds.MariaDbEngineVersion.VER_10_0_17,
            ),
        )

Calling parameter_group.bind_to_cluster() with no arguments fails with:

TypeError: Don't know how to convert object to JSON: ParameterGroupClusterBindOptions()

Calling parameter_group.bind_to_cluster() with either rds.ParameterGroupClusterBindOptions(), or {}, fails with:

TypeError: bind_to_cluster() takes 1 positional argument but 2 were given
@skinny85 skinny85 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 19, 2021
@RomainMuller RomainMuller removed their assignment Jun 24, 2021
@NGL321 NGL321 added cdk-blocker language/python Related to Python bindings p1 and removed needs-triage This issue or PR still needs to be triaged. p2 labels Jul 12, 2021
@NGL321
Copy link
Contributor

NGL321 commented Jul 12, 2021

I have updated priority to match the related CDK issue

@schematis
Copy link

Any ETA on a fix for this?

@RomainMuller
Copy link
Contributor

Ah - the problem boils down to {} being False-y in Python, I think. It's a trivial fix.

RomainMuller added a commit that referenced this issue Feb 8, 2022
Empty dictionaries are false-y in Python, so the test that checked for
structs via the existence of a property mapping table was incorrect, as
it only checked truthiness of the mapping, instead of checking for it
being non-`None`.

Fixes #2846
@mergify mergify bot closed this as completed in #3372 Feb 8, 2022
mergify bot pushed a commit that referenced this issue Feb 8, 2022
Empty dictionaries are false-y in Python, so the test that checked for
structs via the existence of a property mapping table was incorrect, as
it only checked truthiness of the mapping, instead of checking for it
being non-`None`.

Fixes #2846



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. cdk-blocker language/python Related to Python bindings p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants