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

feat: Add pip_tmp_dir to allow setting the location of the pip temporary directory #230

Merged
merged 6 commits into from
Dec 10, 2021
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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ source_path = [
}, {
path = "src/python3.8-app1",
pip_requirements = true,
pip_tmp_dir = "/tmp/dir/location"
prefix_in_zip = "foo/bar1",
}, {
path = "src/python3.8-app2",
Expand Down Expand Up @@ -442,6 +443,7 @@ Few notes:
- `commands` - List of commands to run. If specified, this argument overrides `pip_requirements`.
- `:zip [source] [destination]` is a special command which creates content of current working directory (first argument) and places it inside of path (second argument).
- `pip_requirements` - Controls whether to execute `pip install`. Set to `false` to disable this feature, `true` to run `pip install` with `requirements.txt` found in `path`. Or set to another filename which you want to use instead.
- `pip_tmp_dir` - Set the base directory to make the temporary directory for pip installs. Can be useful for Docker in Docker builds.
- `prefix_in_zip` - If specified, will be used as a prefix inside zip-archive. By default, everything installs into the root of zip-archive.

### Building in Docker
Expand Down
1 change: 1 addition & 0 deletions examples/build-package/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Note that this example may create resources which cost money. Run `terraform des
| <a name="module_lambda_layer"></a> [lambda\_layer](#module\_lambda\_layer) | ../../ | n/a |
| <a name="module_lambda_layer_pip_requirements"></a> [lambda\_layer\_pip\_requirements](#module\_lambda\_layer\_pip\_requirements) | ../.. | n/a |
| <a name="module_package_dir"></a> [package\_dir](#module\_package\_dir) | ../../ | n/a |
| <a name="module_package_dir_pip_dir"></a> [package\_dir\_pip\_dir](#module\_package\_dir\_pip\_dir) | ../../ | n/a |
| <a name="module_package_dir_without_pip_install"></a> [package\_dir\_without\_pip\_install](#module\_package\_dir\_without\_pip\_install) | ../../ | n/a |
| <a name="module_package_file"></a> [package\_file](#module\_package\_file) | ../../ | n/a |
| <a name="module_package_file_with_pip_requirements"></a> [package\_file\_with\_pip\_requirements](#module\_package\_file\_with\_pip\_requirements) | ../../ | n/a |
Expand Down
14 changes: 14 additions & 0 deletions examples/build-package/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,20 @@ module "package_dir" {
source_path = "${path.module}/../fixtures/python3.8-app1"
}

# Create zip-archive of a single directory where "pip install" will also be executed (default for python runtime) and set temporary directory for pip install
artificial-aidan marked this conversation as resolved.
Show resolved Hide resolved
module "package_dir_pip_dir" {
source = "../../"

create_function = false

runtime = "python3.8"
source_path = [{
path = "${path.module}/../fixtures/python3.8-app1"
pip_tmp_dir = "${path.cwd}/../fixtures"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why path.cwd instead of path.module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to remember now, but I believe that path.module has a ../ prefix in it. Which something down the line did not like.

Copy link
Member

Choose a reason for hiding this comment

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

path.module seems to be a better choice here (see docs about path.cwd).

Optionally, user can use something like - abspath(path.module) if absolute path is required.

pip_requirements = "${path.module}/../fixtures/python3.8-app1/requirements.txt"
}]
}

# Create zip-archive of a single directory without running "pip install" (which is default for python runtime)
module "package_dir_without_pip_install" {
source = "../../"
Expand Down
21 changes: 11 additions & 10 deletions package.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ def cd(path, silent=False):


@contextmanager
def tempdir():
def tempdir(dir=None):
"""Creates a temporary directory and then deletes it afterwards."""
prefix = 'terraform-aws-lambda-'
path = tempfile.mkdtemp(prefix=prefix)
path = tempfile.mkdtemp(prefix=prefix, dir=dir)
cmd_log.info('mktemp -d %sXXXXXXXX # %s', prefix, shlex.quote(path))
try:
yield path
Expand Down Expand Up @@ -648,7 +648,7 @@ def plan(self, source_path, query):
step = lambda *x: build_plan.append(x)
hash = source_paths.append

def pip_requirements_step(path, prefix=None, required=False):
def pip_requirements_step(path, prefix=None, required=False, tmp_dir=None):
requirements = path
if os.path.isdir(path):
requirements = os.path.join(path, 'requirements.txt')
Expand All @@ -657,7 +657,7 @@ def pip_requirements_step(path, prefix=None, required=False):
raise RuntimeError(
'File not found: {}'.format(requirements))
else:
step('pip', runtime, requirements, prefix)
step('pip', runtime, requirements, prefix, tmp_dir)
hash(requirements)

def commands_step(path, commands):
Expand Down Expand Up @@ -735,10 +735,10 @@ def commands_step(path, commands):

if pip_requirements and runtime.startswith('python'):
if isinstance(pip_requirements, bool) and path:
pip_requirements_step(path, prefix, required=True)
pip_requirements_step(path, prefix, required=True, tmp_dir=claim.get('pip_tmp_dir'))
else:
pip_requirements_step(pip_requirements, prefix,
required=True)
required=True, tmp_dir=claim.get('pip_tmp_dir'))

if path:
step('zip', path, prefix)
Expand Down Expand Up @@ -784,8 +784,8 @@ def execute(self, build_plan, zip_stream, query):
else:
zs.write_file(source_path, prefix=prefix, timestamp=ts)
elif cmd == 'pip':
runtime, pip_requirements, prefix = action[1:]
with install_pip_requirements(query, pip_requirements) as rd:
runtime, pip_requirements, prefix, tmp_dir = action[1:]
with install_pip_requirements(query, pip_requirements, tmp_dir) as rd:
if rd:
if pf:
self._zip_write_with_filter(zs, pf, rd, prefix,
Expand Down Expand Up @@ -825,7 +825,7 @@ def _zip_write_with_filter(zip_stream, path_filter, source_path, prefix,


@contextmanager
def install_pip_requirements(query, requirements_file):
def install_pip_requirements(query, requirements_file, tmp_dir):
# TODO:
# 1. Emit files instead of temp_dir

Expand All @@ -836,6 +836,7 @@ def install_pip_requirements(query, requirements_file):
runtime = query.runtime
artifacts_dir = query.artifacts_dir
docker = query.docker
temp_dir = query.temp_dir
artificial-aidan marked this conversation as resolved.
Show resolved Hide resolved
docker_image_tag_id = None

if docker:
Expand Down Expand Up @@ -868,7 +869,7 @@ def install_pip_requirements(query, requirements_file):
working_dir = os.getcwd()

log.info('Installing python requirements: %s', requirements_file)
with tempdir() as temp_dir:
with tempdir(tmp_dir) as temp_dir:
requirements_filename = os.path.basename(requirements_file)
target_file = os.path.join(temp_dir, requirements_filename)
shutil.copyfile(requirements_file, target_file)
Expand Down