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
21 changes: 19 additions & 2 deletions aws_lambda_builders/workflows/custom_make/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,16 @@ class CustomMakeAction(BaseAction):
DESCRIPTION = "Running build target on Makefile"
PURPOSE = Purpose.COMPILE_SOURCE

def __init__(self, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_make, build_logical_id):
def __init__(
self,
artifacts_dir,
scratch_dir,
manifest_path,
osutils,
subprocess_make,
build_logical_id,
working_directory=None,
):
"""
:type artifacts_dir: str
:param artifacts_dir: directory where artifacts needs to be stored.
Expand All @@ -38,6 +47,13 @@ def __init__(self, artifacts_dir, scratch_dir, manifest_path, osutils, subproces

:type subprocess_make aws_lambda_builders.workflows.custom_make.make.SubprocessMake
:param subprocess_make: An instance of the Make process wrapper

:type build_logical_id: str
:param build_logical_id: the lambda resource logical id that will be built by the custom action.

:type working_directory: str
:param working_directory: path to the working directory where the Makefile will be executed. Use the scratch_dir
as the working directory if the input working_directory is None
"""
super(CustomMakeAction, self).__init__()
self.artifacts_dir = artifacts_dir
Expand All @@ -46,6 +62,7 @@ def __init__(self, artifacts_dir, scratch_dir, manifest_path, osutils, subproces
self.osutils = osutils
self.subprocess_make = subprocess_make
self.build_logical_id = build_logical_id
self.working_directory = working_directory if working_directory else scratch_dir
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this implied behavior? we should document that in the docstring as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the current default behaviour. Custom builder run against the scratch_dir, so if there is no working_directory passed by the customers, we should use the current default behaviour. I updated the docstring.


@property
def artifact_dir_path(self):
Expand Down Expand Up @@ -91,7 +108,7 @@ def execute(self):
"build-{logical_id}".format(logical_id=self.build_logical_id),
],
env=current_env,
cwd=self.scratch_dir,
cwd=self.working_directory,
)
except MakeExecutionError as ex:
raise ActionFailedError(str(ex))
2 changes: 2 additions & 0 deletions aws_lambda_builders/workflows/custom_make/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
# Find the logical id of the function to be built.
options = kwargs.get("options") or {}
build_logical_id = options.get("build_logical_id", None)
working_directory = options.get("working_directory", scratch_dir)

if not build_logical_id:
raise WorkflowFailedError(
Expand All @@ -51,6 +52,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
osutils=self.os_utils,
subprocess_make=subprocess_make,
build_logical_id=build_logical_id,
working_directory=working_directory,
)

self.actions = [CopySourceAction(source_dir, scratch_dir, excludes=self.EXCLUDED_FILES), make_action]
Expand Down
42 changes: 42 additions & 0 deletions tests/integration/workflows/custom_make/test_custom_make.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,48 @@ def test_must_build_python_project_through_makefile(self):
output_files = set(os.listdir(self.artifacts_dir))
self.assertEqual(expected_files, output_files)

def test_build_python_project_failed_through_makefile_no_python_source_in_default_working_directory(self):
source_code = os.path.join(os.path.dirname(__file__), "testdata", "makefile-in-different-working-directory")
manifest_path_valid = os.path.join(source_code, "Makefile")
with self.assertRaises(WorkflowFailedError):
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this test fail and raise a WorkflowFailedError? working_directory is an optional param right?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I think it's because the requirements file is inside source_code, not in makefile-in-different-working-directory

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, I had a call with Sriram to explain that but forgot to reply to this comment

self.builder.build(
source_code,
self.artifacts_dir,
self.scratch_dir,
manifest_path_valid,
runtime=self.runtime,
options={"build_logical_id": "HelloWorldFunction"},
)

def test_must_build_python_project_through_makefile_with_custom_working_directory(self):
source_code = os.path.join(os.path.dirname(__file__), "testdata", "makefile-in-different-working-directory")
manifest_path_valid = os.path.join(source_code, "Makefile")
working_directory = os.path.join(source_code, "source_code")
self.builder.build(
source_code,
self.artifacts_dir,
self.scratch_dir,
manifest_path_valid,
runtime=self.runtime,
options={"build_logical_id": "HelloWorldFunction", "working_directory": working_directory},
)
dependencies_installed = {
"chardet",
"urllib3",
"idna",
"urllib3-1.25.11.dist-info",
"chardet-3.0.4.dist-info",
"certifi-2020.4.5.2.dist-info",
"certifi",
"idna-2.10.dist-info",
"requests",
"requests-2.23.0.dist-info",
}

expected_files = self.test_data_files.union(dependencies_installed)
output_files = set(os.listdir(self.artifacts_dir))
self.assertEqual(expected_files, output_files)

def test_must_build_python_project_through_makefile_unknown_target(self):
with self.assertRaises(WorkflowFailedError):
self.builder.build(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
build-HelloWorldFunction:
cp *.py $(ARTIFACTS_DIR)
cp requirements-requests.txt $(ARTIFACTS_DIR)
python -m pip install -r requirements-requests.txt -t $(ARTIFACTS_DIR)
rm -rf $(ARTIFACTS_DIR)/bin
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import requests


def lambda_handler(event, context):
# Just return the requests version.
return "{}".format(requests.__version__)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
requests==2.23.0

# Pinning so the test can expect a given version
certifi==2020.4.5.2 # dep of requests
26 changes: 26 additions & 0 deletions tests/unit/workflows/custom_make/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,32 @@ def test_call_makefile_target(self, OSUtilMock, SubprocessMakeMock):
["--makefile", "manifest", "build-logical_id"], cwd="scratch_dir", env=ANY
)

@patch("aws_lambda_builders.workflows.custom_make.utils.OSUtils")
@patch("aws_lambda_builders.workflows.custom_make.make.SubProcessMake")
def test_call_makefile_target_with_working_directory(self, OSUtilMock, SubprocessMakeMock):
osutils = OSUtilMock.return_value
subprocess_make = SubprocessMakeMock.return_value

action = CustomMakeAction(
"artifacts",
"scratch_dir",
"manifest",
osutils=osutils,
subprocess_make=subprocess_make,
build_logical_id="logical_id",
working_directory="working_dir",
)

osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b)

action.execute()

subprocess_make.run.assert_called_with(
["--makefile", "manifest", "build-logical_id"], cwd="working_dir", env=ANY
)

@patch("aws_lambda_builders.workflows.custom_make.utils.OSUtils")
@patch("aws_lambda_builders.workflows.custom_make.make.SubProcessMake")
def test_makefile_target_fails(self, OSUtilMock, SubprocessMakeMock):
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/workflows/custom_make/test_workflow.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from unittest import TestCase
from unittest.mock import patch

from aws_lambda_builders.architecture import X86_64, ARM64
from aws_lambda_builders.actions import CopySourceAction
Expand Down Expand Up @@ -46,3 +47,20 @@ def test_must_validate_architecture(self):

self.assertEqual(workflow.architecture, "x86_64")
self.assertEqual(workflow_with_arm.architecture, "arm64")

def test_use_scratch_dir_as_working_directory(self):
workflow = CustomMakeWorkflow(
"source", "artifacts", "scratch_dir", "manifest", options={"build_logical_id": "hello"}
)
self.assertEqual(workflow.actions[1].working_directory, "scratch_dir")

def test_use_working_directory(self):
workflow = CustomMakeWorkflow(
"source",
"artifacts",
"scratch_dir",
"manifest",
options={"build_logical_id": "hello", "working_directory": "working/dir/path"},
)

self.assertEqual(workflow.actions[1].working_directory, "working/dir/path")