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

Disable Python Argument Lifting #4721

Open
2 tasks
rehanvdm opened this issue Dec 3, 2024 · 1 comment
Open
2 tasks

Disable Python Argument Lifting #4721

rehanvdm opened this issue Dec 3, 2024 · 1 comment
Labels
effort/medium Medium work item – a couple days of effort feature-request A feature should be added or improved. p2

Comments

@rehanvdm
Copy link

rehanvdm commented Dec 3, 2024

Describe the feature

An option to disable Python argument lifting for the whole project or specific functions and constructors.

Use Case

I need help turning "argument lifting in Python" off. By chance, I found some mention of this in the docs as I did not even know how to ask this question. https://aws.github.io/jsii/user-guides/language-support

It will automatically "deconstruct" the properties of the last argument if that argument is an interface.
The docs say it applies to constructors but it applies to any function it seems.
Let's take this example. I have two functions

  public static async diffAll(props: DataLandingZoneProps) {
    return diff.all(props);
  }
  public static async diffSelect(props: DataLandingZoneProps, id: string) {
    return diff.select(props, id);
  }

For diffAll the last (and only) argument is an interface. So JSII decides to deconstruct/argument lift (help me with the correct word here?, not a Python dev) and when calling it wants you to pass in each of the properties.

    def diff_all(
        cls,
        *,
        budgets: typing.Sequence[typing.Union[DlzBudgetProps, typing.Dict[builtins.str, typing.Any]]],
        local_profile: builtins.str,
        mandatory_tags: typing.Union[MandatoryTags, typing.Dict[builtins.str, typing.Any]],
        organization: typing.Union[DLzOrganization, typing.Dict[builtins.str, typing.Any]],
        regions: typing.Union[DlzRegions, typing.Dict[builtins.str, typing.Any]],
        security_hub_notifications: typing.Sequence[typing.Union["SecurityHubNotification", typing.Dict[builtins.str, typing.Any]]],
        additional_mandatory_tags: typing.Optional[typing.Sequence[typing.Union[DlzTag, typing.Dict[builtins.str, typing.Any]]]] = None,
        default_notification: typing.Optional[typing.Union[NotificationDetailsProps, typing.Dict[builtins.str, typing.Any]]] = None,
        deny_service_list: typing.Optional[typing.Sequence[builtins.str]] = None,
        deployment_platform: typing.Optional[typing.Union[DeploymentPlatform, typing.Dict[builtins.str, typing.Any]]] = None,
        iam_identity_center: typing.Optional[typing.Union[IamIdentityCenterProps, typing.Dict[builtins.str, typing.Any]]] = None,
        iam_policy_permission_boundary: typing.Optional[typing.Union[IamPolicyPermissionsBoundaryProps, typing.Dict[builtins.str, typing.Any]]] = None,
        network: typing.Optional[typing.Union[Network, typing.Dict[builtins.str, typing.Any]]] = None,
        print_deployment_order: typing.Optional[builtins.bool] = None,
        print_report: typing.Optional[builtins.bool] = None,
        save_report: typing.Optional[builtins.bool] = None,
    ) -> None:

For diffSelect the last argument is not an interface, so it leaves the first argument that is an interface alone and does not expand it.

def diff_select(
        cls,
        props: typing.Union[DataLandingZoneProps, typing.Dict[builtins.str, typing.Any]],
        id: builtins.str,
    ) -> None:

How can I stop this argument lifting? I peeked at the source (the getliftedProp function is defined here) and it does not seem an option like this exists?

Proposed Solution

No response

Other Information

No response

Acknowledgements

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

CDK version used

Environment details (OS name and version, etc.)

@rehanvdm rehanvdm added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 3, 2024
@rehanvdm
Copy link
Author

rehanvdm commented Dec 3, 2024

I can trick it by making the last argument an empty interface like this:

export interface ForceNoPythonArgumentLifting {}

  /**
   * CDK diff all stacks
   * @param props
   * @param _ Ignore this parameter, it is used to force a consistent interface across TS and Python usage
   */
  public async diffAll(props: DataLandingZoneProps, _: ForceNoPythonArgumentLifting = {}) {
    return diff.all(props);
  }

Then the generated Python is correct:

    def diff_all(
        self,
        props: typing.Union[DataLandingZoneProps, typing.Dict[builtins.str, typing.Any]],
    ) -> None:

And it can be called the same as TS:

import aws_data_landing_zone as dlz
from config import config

scripts = dlz.Scripts()
scripts.diff_all(config)

@otaviomacedo otaviomacedo added effort/medium Medium work item – a couple days of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Dec 16, 2024
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. p2
Projects
None yet
Development

No branches or pull requests

2 participants