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

Update cdktf and use prebuilt providers #2328

Conversation

DanielMSchmidt
Copy link
Contributor

Hey, thanks for this awesome project, I'm really keen to try it out!

  • Updates cdktf to 0.9 and adds every used provider as prebuilt provider @cdktf/provider-...
  • Moves from TerraformStack to Construct for base class so that the class can be used within a Terraform Stack
  • Adds keyword so this appears on constructs.dev

Feel free to let me know if I should drop any of the commits :)

CDKTF does not support nested stacks, therfore would the current solution only work if one directly uses the construct as a stack
@slsnextbot
Copy link
Collaborator

Handler Size Report

No changes to handler sizes.

Base Handler Sizes (kB) (commit b975aed)

{
    "Lambda": {
        "Default Lambda": {
            "Standard": 1524,
            "Minified": 668
        },
        "Image Lambda": {
            "Standard": 1488,
            "Minified": 800
        }
    },
    "Lambda@Edge": {
        "Default Lambda": {
            "Standard": 1534,
            "Minified": 673
        },
        "Default Lambda V2": {
            "Standard": 1526,
            "Minified": 670
        },
        "API Lambda": {
            "Standard": 634,
            "Minified": 318
        },
        "Image Lambda": {
            "Standard": 1496,
            "Minified": 805
        },
        "Regeneration Lambda": {
            "Standard": 1187,
            "Minified": 546
        },
        "Regeneration Lambda V2": {
            "Standard": 1253,
            "Minified": 573
        }
    }
}

New Handler Sizes (kB) (commit 00aa315)

{
    "Lambda": {
        "Default Lambda": {
            "Standard": 1524,
            "Minified": 668
        },
        "Image Lambda": {
            "Standard": 1488,
            "Minified": 800
        }
    },
    "Lambda@Edge": {
        "Default Lambda": {
            "Standard": 1534,
            "Minified": 673
        },
        "Default Lambda V2": {
            "Standard": 1526,
            "Minified": 670
        },
        "API Lambda": {
            "Standard": 634,
            "Minified": 318
        },
        "Image Lambda": {
            "Standard": 1496,
            "Minified": 805
        },
        "Regeneration Lambda": {
            "Standard": 1187,
            "Minified": 546
        },
        "Regeneration Lambda V2": {
            "Standard": 1253,
            "Minified": 573
        }
    }
}

@codecov
Copy link

codecov bot commented Jan 31, 2022

Codecov Report

Merging #2328 (00aa315) into master (b975aed) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2328   +/-   ##
=======================================
  Coverage   83.55%   83.55%           
=======================================
  Files         102      102           
  Lines        3679     3679           
  Branches     1176     1176           
=======================================
  Hits         3074     3074           
  Misses        593      593           
  Partials       12       12           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b975aed...00aa315. Read the comment docs.

@dphang
Copy link
Collaborator

dphang commented Feb 8, 2022

Sorry i didn't see it for a while. This looks great! I've been meaning to complete the reference cdktf implementation for lambda, so does it work better out of the box now?

@DanielMSchmidt
Copy link
Contributor Author

So does it work better out of the box now?

I haven't tested it to be honest, I just looked at the code and made the adjustments and checked it synthed. What works better now is dependency management, if someone already uses aws and the prebuilt provider they have one version installed, so the one used here is not conflicting e.g. in the provider version. Plus you can install multiple apps in one installation :)

@dphang dphang merged commit 47642bf into serverless-nextjs:master Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants