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

fix(python): unable to override methods with keyword arguments #3695

Merged
merged 2 commits into from
Aug 2, 2022

Conversation

RomainMuller
Copy link
Contributor

The callback logic in the jsii runtime library for Python was
incorrectly passing keyword arguments to Python implementations,
forwarding a struct instance as-is instead of destructuring it into a
keyword arguments mapping.

Using inspect.signature(), detect the presence of keyword arguments on
the callback target, and destructure the struct as relevant.

Fixes #3656


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

The callback logic in the jsii runtime library for Python was
incorrectly passing keyword arguments to Python implementations,
forwarding a struct instance as-is instead of destructuring it into a
keyword arguments mapping.

Using `inspect.signature()`, detect the presence of keyword arguments on
the callback target, and destructure the struct as relevant.

Fixes #3656
@RomainMuller RomainMuller requested a review from a team August 2, 2022 12:31
@RomainMuller RomainMuller self-assigned this Aug 2, 2022
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Aug 2, 2022
Comment on lines +1423 to +1431
},
{
"login": "gshpychka",
"name": "Glib Shpychka",
"avatar_url": "https://avatars.githubusercontent.com/u/23005347?v=4",
"profile": "https://github.com/gshpychka",
"contributions": [
"bug"
]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.02$: This change seems unrelated to the fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the record for the individual who's reported this bug... Need I submit 2 separate PRs?

@mergify
Copy link
Contributor

mergify bot commented Aug 2, 2022

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Aug 2, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 2, 2022

Merging (with squash)...

@mergify mergify bot merged commit bcffb4b into main Aug 2, 2022
@mergify mergify bot deleted the rmuller/py-callback-kwargs branch August 2, 2022 15:03
@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python: cannot extend generated classes in an idiomatic way
2 participants