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

(aws_s3_assets): unable to define bundling with local (python) #17928

Closed
andrewjroth opened this issue Dec 9, 2021 · 5 comments · Fixed by aws/jsii#3280
Closed

(aws_s3_assets): unable to define bundling with local (python) #17928

andrewjroth opened this issue Dec 9, 2021 · 5 comments · Fixed by aws/jsii#3280
Assignees
Labels
@aws-cdk/aws-s3-assets documentation This is a problem with documentation. feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.

Comments

@andrewjroth
Copy link

link to reference doc page

https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_s3_assets/README.html#asset-bundling

Describe your issue?

The documentation has an example which is not syntactically correct for Python:

assets.Asset(self, "BundledAsset",
    path="/path/to/asset",
    bundling={
        "local": {
            def try_bundle(self, output_dir, options):
                if can_run_locally:
                    # perform local bundling here
                    return True
                return False
        },
        # Docker bundling fallback
        "image": DockerImage.from_registry("alpine"),
        "entrypoint": ["/bin/sh", "-c"],
        "command": ["bundle"]
    }
)

The value for 'bundling' isn't correct Python code. You can't define a function in the middle of a dictionary.

I've attempted to determine how to correctly define this value, but haven't been able to come up with the correct syntax. Based on the documentation, specific types are expected, so I tried constructing those types ahead of time:

class JinjaLocalBundling(ILocalBundling):
    def try_bundle(self, output_dir, options, *args, **kwargs) -> bool:
        print("Local Jinja2 Render:", self, output_dir, options, args, kwargs)
        return False

JinjaBundling = BundlingOptions(
    local=JinjaLocalBundling,
    image=DockerImage.from_registry("python3"),
    command=["python3 -m pip install jinja2"]
)

Then, including it in the Asset creation:

Asset(self, "BundleTest", path=os.path.join(dirname, "configure.jinja"),
      bundling=JinjaBundling)

This results in an error:

TypeError: Don't know how to convert object to JSON: <class 'test_app.test_stack.JinjaLocalBundling'>

There is very little documentation on how to do this correctly. Can someone demonstrate the correct way to use the bundling option for Asset definitions?

Overall Goal: I want to render a script to be executed as part of the EC2 User Data by replacing variables in the script with values from the stack. For example, I have a script which syncs data from an S3 bucket, but the S3 bucket name needs to be injected into the script. Something like this to replace {{ bucketname }} with the bucket defined by the CDK app:

aws s3 sync s3://{{ bucketname }}/path/ /tmp/path/

If there is an easier way to do this, please let me know. (but also fix the doc since it really doesn't work)

Thanks!!

@andrewjroth andrewjroth added documentation This is a problem with documentation. feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Dec 9, 2021
@otaviomacedo
Copy link
Contributor

Can you try:

@jsii.implements(ILocalBundling)
class JinjaLocalBundling()
    def try_bundle(...)

and then pass the instance of this class to BundlingOptions:

JinjaBundling = BundlingOptions(
    local=JinjaLocalBundling(),
    ...

@andrewjroth
Copy link
Author

Thanks for the suggestion, but that doesn't seem to work for me...

@jsii.implements(ILocalBundling)
class JinjaLocalBundling(ILocalBundling):
    def try_bundle(self, output_dir, options, *args, **kwargs) -> bool:
        print("Local Jinja2 Render:", self, output_dir, options, args, kwargs)
        return False

JinjaBundling = BundlingOptions(
    local=JinjaLocalBundling(),
    image=DockerImage.from_registry("python3"),
    command=["python3 -m pip install jinja2"]
)

Within Stack class:

asset = Asset(self, "ConfigureBundle", path=os.path.join(dirname, "configure.sh"),
                   bundling=JinjaBundling)

Result of cdk synth:

TypeError: Don't know how to convert object to JSON: <app.app_stack.JinjaLocalBundling object at 0x7fb8eb2aba90>

It seems that JSII wants to convert the local parameter to BundlingOptions to JSON for some reason. If so, I'm not sure what JSON I could provide to match the signature and define the try _bundle function.

Again, the documentation needs to have a valid example.

@rix0rrr
Copy link
Contributor

rix0rrr commented Dec 20, 2021

Try replacing:

@jsii.implements(ILocalBundling)
class JinjaLocalBundling(ILocalBundling):

with

@jsii.implements(ILocalBundling)
class JinjaLocalBundling:

?

This should be the syntax, based on the tests here: https://github.com/aws/jsii/blob/main/packages/%40jsii/python-runtime/tests/test_compliance.py#L172

rix0rrr added a commit to aws/jsii that referenced this issue Dec 21, 2021
For Python, interface implementation needs to be written as
`@jsii.implements(Iface)`.

Also add a check that people don't put functions in object literals.

Fixes aws/aws-cdk#17928
mergify bot pushed a commit to aws/jsii that referenced this issue Dec 31, 2021
For Python, interface implementation needs to be written as
`@jsii.implements(Iface)`.

Also add a check that people don't put functions in object literals.

Fixes aws/aws-cdk#17928



---

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

⚠️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.

@alexpulver
Copy link
Contributor

alexpulver commented Sep 5, 2022

@rix0rrr the aws_s3_assets package overview documentation for Python generates the following signature for try_bundle method:

def try_bundle(self, output_dir, *, image, entrypoint=None, command=None, volumes=None, environment=None, workingDirectory=None, user=None, local=None, outputType=None, securityOpt=None, network=None):

Per my test and TypeScript documentation, it should be:

def try_bundle(self, output_dir, options):

Should I open a separate issue, or would you like to reopen this one?

Full working example:

import aws_cdk.aws_s3_assets as assets
import jsii
from aws_cdk import BundlingOptions, DockerImage, ILocalBundling

@jsii.implements(ILocalBundling)
class MyBundle:
    def try_bundle(self, output_dir, options):
        can_run_locally = True  # replace with actual logic
        if can_run_locally:
            # perform local bundling here
            return True
        return False

assets.Asset(
    self, "BundledAsset",
    path=str(pathlib.Path(__file__).parent.parent.joinpath("docs").resolve()),
    bundling=BundlingOptions(
        local=MyBundle(),
        # Docker bundling fallback
        image=DockerImage.from_registry("alpine"),
        entrypoint=["/bin/sh", "-c"],
        command=["bundle"]
    )
)

cc: @heitorlessa

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3-assets documentation This is a problem with documentation. feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants