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

Python: Ec2Service.placeSpreadAccross example is broken #483

Closed
rix0rrr opened this issue Apr 25, 2019 · 5 comments · Fixed by #513
Closed

Python: Ec2Service.placeSpreadAccross example is broken #483

rix0rrr opened this issue Apr 25, 2019 · 5 comments · Fixed by #513
Assignees
Labels
bug This issue is a bug. language/python Related to Python bindings p0

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 25, 2019

The example https://github.com/aws-samples/aws-cdk-examples/tree/master/python/ecs/ecs-service-with-task-placement is broken in Python:

service.place_spread_across(ecs.BuiltInAttributes.AVAILABILITY_ZONE)

leads to

npx: installed 271 in 3.688s
Traceback (most recent call last):
  File "app.py", line 50, in <module>
    service.place_spread_across([ecs.BuiltInAttributes.AVAILABILITY_ZONE])
  File "/tmp/.venv/lib/python3.6/site-packages/aws_cdk/aws_ecs/__init__.py", line 2683, in place_spread_across
    return jsii.invoke(self, "placeSpreadAcross", [fields])
  File "/tmp/.venv/lib/python3.6/site-packages/jsii/_kernel/__init__.py", line 104, in wrapped
    return _recursize_dereference(kernel, fn(kernel, *args, **kwargs))
  File "/tmp/.venv/lib/python3.6/site-packages/jsii/_kernel/__init__.py", line 258, in invoke
    args=_make_reference_for_native(self, args),
  File "/tmp/.venv/lib/python3.6/site-packages/jsii/_kernel/__init__.py", line 115, in _make_reference_for_native
    return [_make_reference_for_native(kernel, i) for i in d]
  File "/tmp/.venv/lib/python3.6/site-packages/jsii/_kernel/__init__.py", line 115, in <listcomp>
    return [_make_reference_for_native(kernel, i) for i in d]
  File "/tmp/.venv/lib/python3.6/site-packages/jsii/_kernel/__init__.py", line 121, in _make_reference_for_native
    d.__jsii__type__ = "Object"
AttributeError: 'tuple' object has no attribute '__jsii__type__'

Method is variadic, is that the problem? Or it might be about the constant definition.

https://alpha-docs-aws.amazon.com/CDK/api/preview/docs/@aws-cdk_aws-ecs.Ec2Service.html#placespreadacrossfields-void

@rix0rrr rix0rrr added bug This issue is a bug. p0 language/python Related to Python bindings labels Apr 25, 2019
@danlaramay
Copy link

danlaramay commented Apr 29, 2019

I'm getting the same error when using add_user_data from ec2_autoscaling.AutoScalingGroup

  File "app.py", line 13, in <module>
    App1Stack(app, "aws-cdk-example3-app1", vpc_stack.vpc)
  File "/Users/test/.pyenv/versions/3.7.2/lib/python3.7/site-packages/jsii/_runtime.py", line 66, in __call__
    inst = super().__call__(*args, **kwargs)
  File "/private/tmp/example3/stacks/app1.py", line 38, in __init__
    "echo '<?php phpinfo(); ?>' > /var/www/html/phpinfo.php\n")
  File "/private/tmp/example3/venv/lib/python3.7/site-packages/aws_cdk/aws_autoscaling/__init__.py", line 2310, in add_user_data
    return jsii.invoke(self, "addUserData", [script_lines])
  File "/Users/test/.pyenv/versions/3.7.2/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 104, in wrapped
    return _recursize_dereference(kernel, fn(kernel, *args, **kwargs))
  File "/Users/test/.pyenv/versions/3.7.2/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 258, in invoke
    args=_make_reference_for_native(self, args),
  File "/Users/test/.pyenv/versions/3.7.2/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 115, in _make_reference_for_native
    return [_make_reference_for_native(kernel, i) for i in d]
  File "/Users/test/.pyenv/versions/3.7.2/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 115, in <listcomp>
    return [_make_reference_for_native(kernel, i) for i in d]
  File "/Users/test/.pyenv/versions/3.7.2/lib/python3.7/site-packages/jsii/_kernel/__init__.py", line 121, in _make_reference_for_native
    d.__jsii__type__ = "Object"
AttributeError: 'tuple' object has no attribute '__jsii__type__'```

@garnaat
Copy link
Contributor

garnaat commented Apr 29, 2019

Taking a look now.

@garnaat garnaat self-assigned this Apr 29, 2019
@carterv
Copy link

carterv commented May 6, 2019

I ran into this same error when using aws_cdk.aws_sqs.Queue.grant(). It appears that the variadic args end up being passed into jsii._kernel._make_reference_for_native() as a tuple, which falls to the default handling of object. The solution would be to change isinstance(d, list) to isinstance(d, (list, tuple)) in the function here.

As an aside, fixing that reveals another issue with the grant() function which is probably more appropriately discussed in the aws_cdk repo (the variadic args should be expanded when passed to the invoke method).

@garnaat
Copy link
Contributor

garnaat commented May 7, 2019

I think there are a couple of problems. First, the code generator for variadic args is wrapping the tuple passed to the method in a list, like this:

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

The list gets unpacked and then the tuple contained within it isn't handled properly and causes the above tuple error.

The second problem is that the Python kernel is not handling tuples, only lists. So code like:

def _recursize_dereference(kernel, d):
    if isinstance(d, dict):
        return {k: _recursize_dereference(kernel, v) for k, v in d.items()}
    elif isinstance(d, list):
        return [_recursize_dereference(kernel, i) for i in d]
    elif isinstance(d, ObjRef):
        return _reference_map.resolve_reference(kernel, d)
    elif isinstance(d, EnumRef):
        return _recursize_dereference(kernel, d.ref)(d.member)
    else:
        return d

needs to handle tuples as well as lists.

@carterv
Copy link

carterv commented May 8, 2019

I was unaware that the Python CDK was auto-generated, that's pretty nifty. I guess it also means that the issue I opened on the CDK repo isn't super relevant over there.

Digging into the code generator, it seems like this line in emitJsiiMethodCall() might be the problem:

code.line(`${methodPrefix}jsii.${this.jsiiMethod}(${jsiiMethodParams.join(", ")}, [${paramNames.join(", ")}])`);

I don't have any experience with TypeScript, but this seems like you could solve the unpacking issue like so:

// If the args are variadic, expand the last one
let modifiedParamNames: string[] = paramNames.slice();
if (this.parameters.length >= 1 && this.parameters[this.parameters.length - 1].variadic) {
    modifiedParamNames.push("*" + modifiedParamNames.pop());
}

code.line(`${methodPrefix}jsii.${this.jsiiMethod}(${jsiiMethodParams.join(", ")}, [${modifiedParamNames.join(", ")}])`);

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. language/python Related to Python bindings p0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants