feat(lambda-nodejs): use docker instead of npm for parcel-bundler #7169

AlexCheema · 2020-04-03T20:35:04Z

Commit Message

feat(lambda-nodejs): use docker instead of npm package for parcel-bundler

End Commit Message

There was a dependency on the parcel-bundler npm package for using the @aws-cdk/aws-lambda-nodejs package. This required installing parcel-bundler in your project. This replaces the npm dependency with a docker image.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…dler

aws-cdk-automation · 2020-04-03T20:42:02Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: ea0e032
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…arcel-bundler

aws-cdk-automation · 2020-04-03T21:07:32Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 3f9db92
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

packages/@aws-cdk/aws-lambda-nodejs/parcel-bundler/Dockerfile

packages/@aws-cdk/aws-lambda-nodejs/README.md

Co-Authored-By: Jonathan Goldwasser <jogold@users.noreply.github.com>

…arcel-bundler

aws-cdk-automation · 2020-04-04T12:31:43Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 4490601
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-04T12:32:56Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 157281e
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-04T12:33:15Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 74383e2
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb

Looks good. I would like us to look into extracting this common code into a separate module like @aws-cdk/aws-lambda-bundling. We can do that in a subsequent PR.

packages/@aws-cdk/aws-lambda-nodejs/README.md

eladb · 2020-04-05T12:08:18Z

packages/@aws-cdk/aws-lambda-nodejs/test/builder.test.ts

@@ -67,15 +67,3 @@ test('throws if status is not 0', () => {
  });
  expect(() => builder.build()).toThrow('status-error');
 });
-
-test('throws when parcel-bundler is not 1.x', () => {


Add a test for when docker is not installed.

eladb · 2020-04-05T12:09:00Z

packages/@aws-cdk/aws-lambda-nodejs/parcel-bundler/Dockerfile

@@ -0,0 +1,5 @@
+FROM node:13.8.0-alpine3.11


Add a wee comment explaining what this docker image is

jogold · 2020-04-05T13:45:06Z

packages/@aws-cdk/aws-lambda-nodejs/parcel-bundler/Dockerfile

@@ -0,0 +1,5 @@
+FROM node:13.8.0-alpine3.11


Suggested change

FROM node:13.8.0-alpine3.11

ARG VERSION=13.8.0-alpine3.11

FROM node:$VERSION

With a prop allowing to customize the version? and the default either in the Dockerfile as above or in the .ts file.

Can we expose this build arg as an optional prop in NodejsFunctionProps and then BuilderOptions? There's a risk of being stucked with a parcel-bundler version that could be incompatible with the fixed node/alpine version of the Dockerfile?

Expose nodeDockerTag in NodejsFunctionProps and pass it to the Builder and we're done here.

eladb · 2020-04-06T11:49:16Z

@jogold LMK if this looks good.

aws-cdk-automation · 2020-04-06T11:51:56Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: e1428ae
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

AlexCheema · 2020-04-06T11:55:59Z

Looks good. I would like us to look into extracting this common code into a separate module like @aws-cdk/aws-lambda-bundling. We can do that in a subsequent PR.

I also wrote an implementation for Python that uses Docker. Feel free to use this when you make the separate module:

import * as cdk from "@aws-cdk/core";
import * as lambda from "@aws-cdk/aws-lambda";
import * as path from "path";
import { spawnSync } from "child_process";

export interface PyFunctionProps {
  /**
   * The runtime environment. Only runtimes of the PYTHON family are
   * supported.
   *
   * @default - PYTHON_3_8
   */
  readonly runtime?: lambda.Runtime;
  /**
   * The path to the entry file.
   */
  readonly entry: string;
  /**
   * The name of the exported handler in the entry file.
   *
   * @default handler
   */
  readonly handler?: string;
  /**
   * The layer directory.
   *
   * @default - `.layer` in the entry file directory
   */
  readonly layerDir?: string;
}
export class PyFunction extends lambda.Function {
  constructor(scope: cdk.Construct, id: string, props: PyFunctionProps) {
    if (props.runtime && props.runtime.family !== lambda.RuntimeFamily.PYTHON) {
      throw new Error("Only `PYTHON` runtimes are supported.");
    }

    const runtime = props.runtime || lambda.Runtime.PYTHON_3_8;
    const entry = props.entry;
    const handler = props.handler || "handler";
    const handlerDir = path.dirname(entry);
    const layerDir = props.layerDir || path.join(handlerDir, ".layer");

    super(scope, id, {
      ...props,
      code: lambda.Code.fromAsset(handlerDir),
      runtime,
      handler: `${path.basename(entry).replace(".py", "")}.${handler}`,
    });

    const dependenciesLayer = createDependenciesLayer(this, runtime, `${handlerDir}/requirements.txt`, layerDir);
    this.addLayers(dependenciesLayer);
  }
}

function createDependenciesLayer(
  scope: cdk.Construct,
  runtime: lambda.Runtime,
  requirementsPath: string,
  layerDir: string,
  skipPip: boolean = false
) {
  if (!skipPip) {
    const dockerArgs = [
      "run",
      "--rm",
      "-v",
      `${path.resolve(requirementsPath)}:/requirements.txt`,
      "-v",
      `${path.resolve(layerDir)}/python:/output`,
      pyV(runtime) === 2 ? "2.7.17-slim-stretch" : "python:3.7.7-slim-stretch",
    ];
    const pipArgs = ["pip", "install", "-r", "/requirements.txt", "-t", "/output"];
    const proc = spawnSync("docker", [...dockerArgs, ...pipArgs]);

    if (proc.error) {
      throw proc.error;
    }

    if (proc.status !== 0) {
      throw new Error(`[Status ${proc.status}] stdout: ${proc.stdout.toString().trim()}\n\n\nstderr: ${proc.stderr.toString().trim()}`);
    }
  }

  return new lambda.LayerVersion(scope, "DependenciesLayer", { code: lambda.Code.fromAsset(layerDir) });
}

function pyV(runtime: lambda.Runtime): 2 | 3 {
  if (runtime.name.includes("python2")) return 2;

  return 3;
}

eladb · 2020-04-06T11:59:41Z

I also wrote an implementation for Python that uses Docker. Feel free to use this when you make the separate module:

Would you like to contribute that?

jogold · 2020-04-06T12:05:55Z

@eladb looks like Docker (docker-in-docker) is not available in the CodeBuild image that is running for PRs?

AlexCheema · 2020-04-06T12:15:46Z

I also wrote an implementation for Python that uses Docker. Feel free to use this when you make the separate module:

Would you like to contribute that?

I can't commit the time to that unfortunately. Feel free to use the code though!

aws-cdk-automation · 2020-04-06T12:21:50Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9a5cb5c
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-06T12:34:20Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 370778d
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…/AlexCheema/aws-cdk into alexcheema/docker-parcel-bundler

eladb · 2020-04-06T12:48:08Z

@eladb looks like Docker (docker-in-docker) is not available in the CodeBuild image that is running for PRs?

Damn! @RomainMuller any suggestion?

aws-cdk-automation · 2020-04-30T12:23:22Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 9f85d87
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-30T12:31:14Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: c7852e9
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-04-30T12:35:18Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: d3c79d4
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb · 2020-04-30T13:01:49Z

@AlexCheema can you update yarn.lock please?

aws-cdk-automation · 2020-04-30T13:14:15Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: feadb67
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…arcel-bundler

AlexCheema · 2020-05-01T06:27:47Z

@AlexCheema can you update yarn.lock please?

Done

aws-cdk-automation · 2020-05-01T06:27:51Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 711b58e
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-05-01T08:11:38Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2020-05-01T08:58:38Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8de8246
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-05-01T08:59:00Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

jogold · 2020-05-01T13:01:11Z

@AlexCheema have you been able to test this with dependencies/modules? I'm always getting Cannot resolve dependency '...' with this setup. Seems like parcel-bundler in the container cannot find those dependencies.

jogold · 2020-05-01T13:11:13Z

I think that this is because the node_modules folder is outside the mounted volume.

@eladb looking into this and waiting for a reply from @AlexCheema but I think that this needs to be reverted/not released...

AlexCheema · 2020-05-02T08:39:46Z

I think that this is because the node_modules folder is outside the mounted volume.

@eladb looking into this and waiting for a reply from @AlexCheema but I think that this needs to be reverted/not released...

I'm not sure I understand the problem. Best if you implement a fix.

Redo of aws#7169.

Redo of #7169.

iDVB · 2020-05-29T15:38:25Z

@eladb @jogold Does NodeJSFunction yet support handling a hash that we can, or it can use for adding function versions? Sorry, I've been bouncing around 4-5 issues to try and catch up on where you guys are at with this feature.

jogold · 2020-05-29T19:33:09Z

@eladb @jogold Does NodeJSFunction yet support handling a hash that we can, or it can use for adding function versions? Sorry, I've been bouncing around 4-5 issues to try and catch up on where you guys are at with this feature.

NodejsFunction extends lambda.Function so everything available on lambda.Function should be available here also, does this answer your question?

iDVB · 2020-05-29T19:35:39Z

@jogold Sorry, I was not more clear. I'm not aware of any process currently that knows if something in your lambda src files changed.
I saw something like this...
lambda.addVersion(sha256(require('./mymain.ts')))
which only works if that ./mymain.ts is a single file. In our case we use NodeJSFunction and is many files that get bundled with parcel.

So is there currently a way to use the output from parcel as the thing to generate the hash from?

feat(lambda-nodejs): use docker instead of npm package for parcel-bun…

ea0e032

…dler

Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…

3f9db92

…arcel-bundler

NGL321 assigned nija-at Apr 3, 2020

jogold reviewed Apr 4, 2020

View reviewed changes

packages/@aws-cdk/aws-lambda-nodejs/parcel-bundler/Dockerfile Outdated Show resolved Hide resolved

jogold reviewed Apr 4, 2020

View reviewed changes

packages/@aws-cdk/aws-lambda-nodejs/README.md Show resolved Hide resolved

AlexCheema and others added 3 commits April 4, 2020 14:24

require version ^1 of parcel-bundler in Dockerfile

4490601

Co-Authored-By: Jonathan Goldwasser <jogold@users.noreply.github.com>

update README: docker requirement

157281e

Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…

74383e2

…arcel-bundler

eladb reviewed Apr 5, 2020

View reviewed changes

jogold reviewed Apr 5, 2020

View reviewed changes

AlexCheema added 2 commits April 6, 2020 13:41

comment in Dockerfile, make node version a build ARG

e744f9c

add test for when docker not installed

e1428ae

add optional nodeDockerTag to props

9a5cb5c

Merge branch 'master' into alexcheema/docker-parcel-bundler

370778d

AlexCheema added 2 commits April 6, 2020 14:43

Expose nodeDockerTag in NodejsFunctionProps and pass it to the Builder

d36a005

Merge branch 'alexcheema/docker-parcel-bundler' of https://github.com…

23ffd92

…/AlexCheema/aws-cdk into alexcheema/docker-parcel-bundler

Elad Ben-Israel added 2 commits April 30, 2020 15:32

Fix docker location.

d3c79d4

Update buildspec.yaml

feadb67

Merge remote-tracking branch 'origin/master' into alexcheema/docker-p…

711b58e

…arcel-bundler

eladb approved these changes May 1, 2020

View reviewed changes

Merge branch 'master' into alexcheema/docker-parcel-bundler

8de8246

mergify bot merged commit 55c4d0b into aws:master May 1, 2020

jogold mentioned this pull request May 1, 2020

revert: "feat(lambda-nodejs): use docker instead of npm for parcel-bundler" #7738

Merged

jogold added a commit to jogold/aws-cdk that referenced this pull request May 7, 2020

feat(lambda-nodejs): run parcel in a docker container

84914ed

Redo of aws#7169.

jogold added a commit to jogold/aws-cdk that referenced this pull request May 7, 2020

feat(lambda-nodejs): run parcel in a docker container

dc9b15d

Redo of aws#7169.

jogold added a commit to jogold/aws-cdk that referenced this pull request May 7, 2020

feat(lambda-nodejs): run parcel in a docker container

c4d4764

Redo of aws#7169.

jogold mentioned this pull request May 7, 2020

feat(lambda-nodejs): run parcel in a docker container #7842

Merged

mergify bot pushed a commit that referenced this pull request May 7, 2020

feat(lambda-nodejs): run parcel in a docker container

d86e500

Redo of #7169.

AlexCheema deleted the alexcheema/docker-parcel-bundler branch May 8, 2020 09:14

blimmer mentioned this pull request May 11, 2020

New Docker-based Parcel Build does not work on CircleCI (and likely other CI platforms) #7912

Closed

	FROM node:13.8.0-alpine3.11
	ARG VERSION=13.8.0-alpine3.11
	FROM node:$VERSION

feat(lambda-nodejs): use docker instead of npm for parcel-bundler #7169

feat(lambda-nodejs): use docker instead of npm for parcel-bundler #7169

Conversation

AlexCheema commented Apr 3, 2020

Commit Message

End Commit Message

aws-cdk-automation commented Apr 3, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 3, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 4, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 4, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 4, 2020

AWS CodeBuild CI Report

eladb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eladb commented Apr 6, 2020

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

AlexCheema commented Apr 6, 2020

eladb commented Apr 6, 2020

jogold commented Apr 6, 2020 • edited

AlexCheema commented Apr 6, 2020

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 6, 2020

AWS CodeBuild CI Report

eladb commented Apr 6, 2020

aws-cdk-automation commented Apr 30, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 30, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Apr 30, 2020

AWS CodeBuild CI Report

eladb commented Apr 30, 2020

aws-cdk-automation commented Apr 30, 2020

AWS CodeBuild CI Report

AlexCheema commented May 1, 2020

aws-cdk-automation commented May 1, 2020

AWS CodeBuild CI Report

mergify bot commented May 1, 2020

aws-cdk-automation commented May 1, 2020

AWS CodeBuild CI Report

mergify bot commented May 1, 2020

jogold commented May 1, 2020 • edited

jogold commented May 1, 2020 • edited

AlexCheema commented May 2, 2020

iDVB commented May 29, 2020

jogold commented May 29, 2020

iDVB commented May 29, 2020 • edited

jogold commented Apr 6, 2020 •

edited

jogold commented May 1, 2020 •

edited

jogold commented May 1, 2020 •

edited

iDVB commented May 29, 2020 •

edited