python: another problem with variadic arguments

[garnaat]

Sorry for the lengthy writeup here. I wanted to provide enough context to enable a discussion about this issue because I'm not 100% sure of the right solution.

There was a previous issue (#483) with variadic arguments in Python. This issue involved methods with scalar variadic parameters such as https://github.com/awslabs/aws-cdk/blob/v0.34.0/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts#L168:

 /**
   * Try to place tasks spread across instance attributes.
   *
   * You can use one of the built-in attributes found on `BuiltInAttributes`
   * or supply your own custom instance attributes. If more than one attribute
   * is supplied, spreading is done in order.
   *
   * @default attributes instanceId
   * @deprecated Use addPlacementStrategies() instead.
   */
  public placeSpreadAcross(...fields: string[]) {
    if (fields.length === 0) {
      this.addPlacementStrategies(PlacementStrategy.spreadAcrossInstances());
    } else {
      this.addPlacementStrategies(PlacementStrategy.spreadAcross(...fields));
    }
  }

This was mapped to a Python signature like this:

 @jsii.member(jsii_name="placeSpreadAcross")
    def place_spread_across(self, *fields: str) -> None:
        return jsii.invoke(self, "placeSpreadAcross", [fields])

The problem was that the tuple containing fields was then passed directly to the Python runtime and it didn't know how to deal with it. The fix was to change the Python code to this:

 @jsii.member(jsii_name="placeSpreadAcross")
    def place_spread_across(self, *fields: str) -> None:
        return jsii.invoke(self, "placeSpreadAcross", [*fields])

This expanded the tuple and fixed the problem. This was accomplished by changing the Python generator code as shown in #513. The change was very simple, just check if the parameter was variadic when generating the call to JSII and, if so, expand the tuple.

                // If the last arg is variadic, expand the tuple
	        const paramNames: string[] = [];
	        for (const param of this.parameters) {
	            paramNames.push((param.variadic ? '*' : '') + toPythonParameterName(param.name));

This change, however, introduced another issue. This issue shows up, among other places, in https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-ecs.ContainerDefinition.html#addportmappingsportmappings-void which looks like this:

 /**
   * Add one or more port mappings to this container
   */
  public addPortMappings(...portMappings: PortMapping[]) {
    this.portMappings.push(...portMappings.map(pm => {
      if (this.taskDefinition.networkMode === NetworkMode.AwsVpc || this.taskDefinition.networkMode === NetworkMode.Host) {
        if (pm.containerPort !== pm.hostPort && pm.hostPort !== undefined) {
          throw new Error(`Host port ${pm.hostPort} does not match container port ${pm.containerPort}.`);
        }
      }

      if (this.taskDefinition.networkMode === NetworkMode.Bridge) {
        if (pm.hostPort === undefined) {
          pm = {
            ...pm,
            hostPort: 0
          };
        }
      }

      return pm;
    }));
  }

In this case the variadic argument is not a scalar value but an object conforming to this interface https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-ecs.PortMapping.html. The generated Python code looks like this:

    @jsii.member(jsii_name="addPortMappings")
    def add_port_mappings(self, *, container_port: jsii.Number, host_port: typing.Optional[jsii.Number]=None, protocol: typing.Optional["Protocol"]=None) -> None:
        """Add one or more port mappings to this container.

        Arguments:
            portMappings: -
            containerPort: Port inside the container.
            hostPort: Port on the host. In AwsVpc or Host networking mode, leave this out or set it to the same value as containerPort. In Bridge networking mode, leave this out or set it to non-reserved non-ephemeral port.
            protocol: Protocol. Default: Tcp

        Stability:
            experimental
        """
        port_mappings: PortMapping = {"containerPort": container_port}

        if host_port is not None:
            port_mappings["hostPort"] = host_port

        if protocol is not None:
            port_mappings["protocol"] = protocol

        return jsii.invoke(self, "addPortMappings", [*port_mappings])

In this case, the parameters are passed as keyword args (the bare asterisk as the first param forces that), the PortMapping structure is built and then wrapped in a list and unpacked. This will result in a list of the keys present in the dict being passed to jsii.invoke, e.g. ['containerPort', 'hostPort'] which results in the following error:

Traceback (most recent call last):
  File "app.py", line 40, in <module>
    protocol=ecs.Protocol.Tcp
  File "/Users/garnaat/projects/cdkdev/aws-cdk-examples/python/ecs/ecs-service-with-task-placement/.env/lib/python3.7/site-packages/aws_cdk/aws_ecs/__init__.py", line 1869, in add_port_mappings
    return jsii.invoke(self, "addPortMappings", [*port_mappings])
  File "/Users/garnaat/projects/cdkdev/aws-cdk-examples/python/ecs/ecs-service-with-task-placement/.env/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 104, in wrapped
    return _recursize_dereference(kernel, fn(kernel, *args, **kwargs))
  File "/Users/garnaat/projects/cdkdev/aws-cdk-examples/python/ecs/ecs-service-with-task-placement/.env/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 258, in invoke
    args=_make_reference_for_native(self, args),
  File "/Users/garnaat/projects/cdkdev/aws-cdk-examples/python/ecs/ecs-service-with-task-placement/.env/lib/python3.7/site-packages/jsii/_kernel/providers/process.py", line 346, in invoke
    return self._process.send(request, InvokeResponse)
  File "/Users/garnaat/projects/cdkdev/aws-cdk-examples/python/ecs/ecs-service-with-task-placement/.env/lib/python3.7/site-packages/jsii/_kernel/providers/process.py", line 316, in send
    raise JSIIError(resp.error) from JavaScriptError(resp.stack)
jsii.errors.JSIIError: Expected object reference, got "containerPort"
Subprocess exited with error 1

Simply removing the * in the call to jsii.invoke fixes the problem but this raises two questions.

1. How to discriminate between scalar and non-scalar arguments in the Python code generator.
2. Isn't this signature for add_port_mappings wrong? You should be able to pass a list of PortMapping objects but that is not possible here.