-
Notifications
You must be signed in to change notification settings - Fork 295
Reduce size of installed dependencies by slimming #191
Changes from 9 commits
91d80c3
76a92b0
7b7c429
4362153
8389e2b
b2d50fa
f500910
82c1088
780af23
81e2663
c406b5f
4302c17
1f9ae75
eeee343
e121d17
f12d600
474b823
d3a6dc8
26216d8
83b170c
671cc72
2230ae4
97212b2
89c46ba
c71be73
33433aa
e6a4273
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,6 +118,12 @@ function installRequirements( | |
| } | ||
| cmdOptions.push(dockerImage); | ||
| cmdOptions.push(...pipCmd); | ||
|
|
||
| // If enabled slimming, strip out the caches, tests and dist-infos | ||
| if (options.slim) { | ||
| const slimCmd = getSlimPackageCommands(options, targetRequirementsFolder); | ||
| cmdOptions.push(...slimCmd); | ||
| } | ||
| } else { | ||
| cmd = pipCmd[0]; | ||
| cmdOptions = pipCmd.slice(1); | ||
|
|
@@ -147,11 +153,29 @@ function installRequirements( | |
| */ | ||
| function dockerPathForWin(options, path) { | ||
| if (process.platform === 'win32' && options.dockerizePip) { | ||
| return path.replace('\\', '/'); | ||
| return path.replace(/\\/g, '/'); | ||
| } | ||
| return path; | ||
| } | ||
|
|
||
| /** | ||
| * get commands to slimmen the installed requirements | ||
| * @param {Object} options | ||
| * @param {string} targetRequirementsFolder | ||
| * @return {Array} | ||
| */ | ||
| function getSlimPackageCommands(options, targetRequirementsFolder) { | ||
| const folderPath = dockerPathForWin(options, targetRequirementsFolder); | ||
| const stripCmd = [ | ||
| `&& find ${folderPath} -name "*.so" -exec strip {} \\;`, | ||
|
||
| `&& find ${folderPath} -name "*.py[c|o]" -exec rm -rf {} +`, | ||
|
||
| `&& find ${folderPath} -type d -name "*tests*" -exec rm -rf {} +`, | ||
| `&& find ${folderPath} -type d -name "__pycache__*" -exec rm -rf {} +`, | ||
| `&& find ${folderPath} -type d -name "*.dist-info*" -exec rm -rf {} +` | ||
| ]; | ||
| return stripCmd; | ||
| } | ||
|
|
||
| /** create a filtered requirements.txt without anything from noDeploy | ||
| * @param {string} source requirements | ||
| * @param {string} target requirements where results are written | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,15 @@ teardown() { | |
| ls puck/.requirements.zip puck/unzip_requirements.py | ||
| } | ||
|
|
||
| @test "py3.6 can package flask with zip & slim & dockerizePip option" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
| ! uname -sm|grep Linux || groups|grep docker || id -u|egrep '^0$' || skip "can't dockerize on linux if not root & not in docker group" | ||
| sls --dockerizePip=true --zip=true --slim=true package | ||
| unzip .serverless/sls-py-req-test.zip -d puck | ||
| ls puck/.requirements.zip puck/unzip_requirements.py | ||
| } | ||
|
|
||
| @test "py3.6 can package flask with dockerizePip option" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
|
|
@@ -67,6 +76,16 @@ teardown() { | |
| ls puck/flask | ||
| } | ||
|
|
||
| @test "py3.6 can package flask with slim & dockerizePip option" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
| ! uname -sm|grep Linux || groups|grep docker || id -u|egrep '^0$' || skip "can't dockerize on linux if not root & not in docker group" | ||
| sls --dockerizePip=true --slim=true package | ||
| unzip .serverless/sls-py-req-test.zip -d puck | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙌 Thanks for including tests too!! Could you add this to also check that the unzipped services doesn't contain any pyc files?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, that's an awesome one, thanks, didn't know of this, will do now! ✌️
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should I put
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup. My mistake. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dee-me-tree-or-love, @dschep Wouldn't you want to dump the .py and not .pyc? |
||
| ls puck/flask | ||
| test $(find puck -name *.pyc | wc -l) -eq 0 | ||
|
||
| } | ||
|
|
||
| @test "py3.6 uses cache with dockerizePip option" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
|
|
@@ -76,6 +95,17 @@ teardown() { | |
| ls .requirements-cache/http | ||
| } | ||
|
|
||
| @test "py3.6 uses cache with dockerizePip & slim option" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
| ! uname -sm|grep Linux || groups|grep docker || id -u|egrep '^0$' || skip "can't dockerize on linux if not root & not in docker group" | ||
| perl -p -i'.bak' -e 's/(pythonRequirements:$)/\1\n pipCmdExtraArgs: ["--cache-dir", ".requirements-cache"]/' serverless.yml | ||
| sls --dockerizePip=true --slim=true package | ||
| ls .requirements-cache/http | ||
| test $(find puck -name *.pyc | wc -l) -eq 0 | ||
| } | ||
|
|
||
|
|
||
| @test "py2.7 can package flask with default options" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
|
|
@@ -118,6 +148,15 @@ teardown() { | |
| ls puck/.requirements.zip puck/unzip_requirements.py | ||
| } | ||
|
|
||
| @test "py2.7 can package flask with zip & slim & dockerizePip option" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
| ! uname -sm|grep Linux || groups|grep docker || id -u|egrep '^0$' || skip "can't dockerize on linux if not root & not in docker group" | ||
| sls --dockerizePip=true --runtime=python2.7 --zip=true --slim=true package | ||
| unzip .serverless/sls-py-req-test.zip -d puck | ||
| ls puck/.requirements.zip puck/unzip_requirements.py | ||
| } | ||
|
|
||
| @test "py2.7 can package flask with dockerizePip option" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
|
|
@@ -127,6 +166,17 @@ teardown() { | |
| ls puck/flask | ||
| } | ||
|
|
||
| @test "py2.7 can package flask with slim & dockerizePip option" { | ||
| cd tests/base | ||
| npm i $(npm pack ../..) | ||
| ! uname -sm|grep Linux || groups|grep docker || id -u|egrep '^0$' || skip "can't dockerize on linux if not root & not in docker group" | ||
| sls --dockerizePip=true --slim=true --runtime=python2.7 package | ||
| unzip .serverless/sls-py-req-test.zip -d puck | ||
| ls puck/flask | ||
| test $(find puck -name *.pyc | wc -l) -eq 0 | ||
| } | ||
|
|
||
|
|
||
| @test "pipenv py3.6 can package flask with default options" { | ||
| cd tests/pipenv | ||
| npm i $(npm pack ../..) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When running tests noticed that setting the
slimparameter via cli as--slim=trueresults in it treated as string.From this
(options.slim === true)was evaluated as false every time and the commands were not executed 😅Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is what you want either then because
Boolean("false")will evaluate totruesince strings are truthy in javascript. I think it should beoptions.slim === true || options.slim === "true"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did the same thing as you for package individually, but David is right, I'd prefer that you use
options.slim === true || options.slim === "true"and I'll update my use of the same as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah! thanks, that's a really good one, commit on the way 👍