Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions aws_lambda_builders/workflows/nodejs_npm/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class NodejsNpmInstallAction(BaseAction):
DESCRIPTION = "Installing dependencies from NPM"
PURPOSE = Purpose.RESOLVE_DEPENDENCIES

def __init__(self, artifacts_dir, subprocess_npm, is_production=True):
def __init__(self, artifacts_dir, subprocess_npm):
"""
:type artifacts_dir: str
:param artifacts_dir: an existing (writable) directory with project source files.
Expand All @@ -96,22 +96,18 @@ def __init__(self, artifacts_dir, subprocess_npm, is_production=True):
super(NodejsNpmInstallAction, self).__init__()
self.artifacts_dir = artifacts_dir
self.subprocess_npm = subprocess_npm
self.is_production = is_production

def execute(self):
"""
Runs the action.

:raises lambda_builders.actions.ActionFailedError: when NPM execution fails
"""

mode = "--production" if self.is_production else "--production=false"

try:
LOG.debug("NODEJS installing in: %s", self.artifacts_dir)

self.subprocess_npm.run(
["install", "-q", "--no-audit", "--no-save", mode, "--unsafe-perm"], cwd=self.artifacts_dir
["install", "-q", "--no-audit", "--no-save", "--unsafe-perm", "--production"], cwd=self.artifacts_dir
)

except NpmExecutionError as ex:
Expand Down
4 changes: 2 additions & 2 deletions aws_lambda_builders/workflows/nodejs_npm/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def get_resolvers(self):
return [PathResolver(runtime=self.runtime, binary="npm")]

@staticmethod
def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils, build_options, is_production=True):
def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils, build_options):
"""
Get the install action used to install dependencies at artifacts_dir

Expand Down Expand Up @@ -180,4 +180,4 @@ def get_install_action(source_dir, artifacts_dir, subprocess_npm, osutils, build
if (osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path)) and npm_ci_option:
return NodejsNpmCIAction(artifacts_dir, subprocess_npm=subprocess_npm)

return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm, is_production=is_production)
return NodejsNpmInstallAction(artifacts_dir, subprocess_npm=subprocess_npm)
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ def esbuild_binary(self):
if binaries:
return binaries[0]
else:
raise EsbuildExecutionError(message="cannot find esbuild")
raise EsbuildExecutionError(
message="Cannot find esbuild. esbuild must be installed on the host machine to use this feature. "
"It is recommended to be installed on the PATH, "
"but can also be included as a project dependency."
)

def run(self, args, cwd=None):

Expand Down
16 changes: 13 additions & 3 deletions aws_lambda_builders/workflows/nodejs_npm_esbuild/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,11 @@ def actions_with_bundler(
)

install_action = NodejsNpmWorkflow.get_install_action(
source_dir, scratch_dir, subprocess_npm, osutils, self.options, is_production=False
source_dir,
scratch_dir,
subprocess_npm,
osutils,
self.options,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume is_production defaults to True?

Is there any reason a customer may want False? If so, how would a customer do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it defaults to true. We don't support any way for customers to set it to False, and we actually don't use it as False anywhere except here so I'm just going to remove that parameter. Setting production=False means that development dependencies will also be installed which adds a ton of bloat and install time that customers likely won't want packaged with their Lambda. Before, we expected customers to include esbuild as a dev dependency. Now, we expect it to be installed on their PATH or as a normal dependency. This is the behaviour we currently have with our Node.js workflow and are making esbuild consistent.

)

if self.download_dependencies and not self.dependencies_dir:
Expand Down Expand Up @@ -188,7 +192,10 @@ def _accelerate_workflow_actions(
actions += esbuild_no_deps
else:
# Invalid workflow, can't have no dependency dir and no installation
raise EsbuildExecutionError(message="Lambda Builders encountered and invalid workflow")
raise EsbuildExecutionError(
message="Lambda Builders encountered an invalid workflow. A workflow can't "
"include a dependencies directory without installing dependencies."
)

return actions

Expand All @@ -211,7 +218,10 @@ def get_resolvers(self):
return [PathResolver(runtime=self.runtime, binary="npm")]

def _get_esbuild_subprocess(self, subprocess_npm, scratch_dir, osutils) -> SubprocessEsbuild:
npm_bin_path = subprocess_npm.run(["bin"], cwd=scratch_dir)
try:
npm_bin_path = subprocess_npm.run(["bin"], cwd=scratch_dir)
except FileNotFoundError:
raise EsbuildExecutionError(message="The esbuild workflow couldn't find npm installed on your system.")
executable_search_paths = [npm_bin_path]
if self.executable_search_paths is not None:
executable_search_paths = executable_search_paths + self.executable_search_paths
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import shutil
import tempfile
from unittest import TestCase
from unittest.mock import patch

from aws_lambda_builders.builder import LambdaBuilder
from aws_lambda_builders.exceptions import WorkflowFailedError
from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm
Expand All @@ -22,10 +24,16 @@ def setUp(self):
self.artifacts_dir = tempfile.mkdtemp()
self.scratch_dir = tempfile.mkdtemp()
self.dependencies_dir = tempfile.mkdtemp()

self.no_deps = os.path.join(self.TEST_DATA_FOLDER, "no-deps-esbuild")

self.builder = LambdaBuilder(language="nodejs", dependency_manager="npm-esbuild", application_framework=None)
self.osutils = OSUtils()
self._set_esbuild_binary_path()

def _set_esbuild_binary_path(self):
npm = SubprocessNpm(self.osutils)
esbuild_dir = os.path.join(self.TEST_DATA_FOLDER, "esbuild-binary")
npm.run(["ci"], cwd=esbuild_dir)
self.binpath = npm.run(["bin"], cwd=esbuild_dir)

def tearDown(self):
shutil.rmtree(self.artifacts_dir)
Expand Down Expand Up @@ -58,6 +66,7 @@ def test_builds_javascript_project_with_dependencies(self, runtime):
os.path.join(source_dir, "package.json"),
runtime=runtime,
options=options,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand All @@ -78,6 +87,7 @@ def test_builds_javascript_project_with_multiple_entrypoints(self, runtime):
os.path.join(source_dir, "package.json"),
runtime=runtime,
options=options,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand All @@ -98,6 +108,7 @@ def test_builds_typescript_projects(self, runtime):
os.path.join(source_dir, "package.json"),
runtime=runtime,
options=options,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand All @@ -107,15 +118,7 @@ def test_builds_typescript_projects(self, runtime):

@parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",)])
def test_builds_with_external_esbuild(self, runtime):
osutils = OSUtils()
npm = SubprocessNpm(osutils)
source_dir = os.path.join(self.TEST_DATA_FOLDER, "no-deps-esbuild")
esbuild_dir = os.path.join(self.TEST_DATA_FOLDER, "esbuild-binary")

npm.run(["ci"], cwd=esbuild_dir)

binpath = npm.run(["bin"], cwd=esbuild_dir)

options = {"entry_points": ["included.js"]}

self.builder.build(
Expand All @@ -125,7 +128,7 @@ def test_builds_with_external_esbuild(self, runtime):
os.path.join(source_dir, "package.json"),
runtime=runtime,
options=options,
executable_search_paths=[binpath],
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand All @@ -144,6 +147,7 @@ def test_no_options_passed_to_esbuild(self, runtime):
self.scratch_dir,
os.path.join(source_dir, "package.json"),
runtime=runtime,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand All @@ -162,6 +166,7 @@ def test_bundle_with_implicit_file_types(self, runtime):
os.path.join(source_dir, "package.json"),
runtime=runtime,
options=options,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand Down Expand Up @@ -237,6 +242,7 @@ def test_builds_project_with_remote_dependencies_with_download_dependencies_and_
options=options,
dependencies_dir=self.dependencies_dir,
download_dependencies=True,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand Down Expand Up @@ -267,10 +273,15 @@ def test_builds_project_with_remote_dependencies_without_download_dependencies_w
runtime=runtime,
dependencies_dir=None,
download_dependencies=False,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

self.assertEqual(str(context.exception), "Esbuild Failed: Lambda Builders encountered and invalid workflow")
self.assertEqual(
str(context.exception),
"Esbuild Failed: Lambda Builders encountered an invalid workflow. A"
" workflow can't include a dependencies directory without installing dependencies.",
)

@parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",)])
def test_builds_project_without_combine_dependencies(self, runtime):
Expand All @@ -287,6 +298,7 @@ def test_builds_project_without_combine_dependencies(self, runtime):
dependencies_dir=self.dependencies_dir,
download_dependencies=True,
combine_dependencies=False,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand Down Expand Up @@ -315,6 +327,7 @@ def test_builds_javascript_project_with_external(self, runtime):
os.path.join(source_dir, "package.json"),
runtime=runtime,
options=options,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand All @@ -340,6 +353,7 @@ def test_builds_javascript_project_with_loader(self, runtime):
os.path.join(source_dir, "package.json"),
runtime=runtime,
options=options,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand Down Expand Up @@ -380,6 +394,7 @@ def test_includes_sourcemap_if_requested(self, runtime):
os.path.join(source_dir, "package.json"),
runtime=runtime,
options=options,
executable_search_paths=[self.binpath],
experimental_flags=[EXPERIMENTAL_FLAG_ESBUILD],
)

Expand Down
Loading