Skip to content

feat: Support individual packaging with poetry #682

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

Merged
merged 12 commits into from
Mar 14, 2022
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ dist/
downloads/
eggs/
.eggs/
lib/
lib64/
parts/
sdist/
Expand Down
2 changes: 0 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const { injectAllRequirements } = require('./lib/inject');
const { layerRequirements } = require('./lib/layer');
const { installAllRequirements } = require('./lib/pip');
const { pipfileToRequirements } = require('./lib/pipenv');
const { pyprojectTomlToRequirements } = require('./lib/poetry');
const { cleanup, cleanupCache } = require('./lib/clean');
BbPromise.promisifyAll(fse);

Expand Down Expand Up @@ -203,7 +202,6 @@ class ServerlessPythonRequirements {
}
return BbPromise.bind(this)
.then(pipfileToRequirements)
.then(pyprojectTomlToRequirements)
.then(addVendorHelper)
.then(installAllRequirements)
.then(packRequirements)
Expand Down
23 changes: 7 additions & 16 deletions lib/pip.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const spawn = require('child-process-ext/spawn');
const { quote } = require('shell-quote');
const { buildImage, getBindPath, getDockerUid } = require('./docker');
const { getStripCommand, getStripMode, deleteFiles } = require('./slim');
const { isPoetryProject } = require('./poetry');
const { isPoetryProject, pyprojectTomlToRequirements } = require('./poetry');
const {
checkForAndDeleteMaxCacheVersions,
sha256Path,
Expand Down Expand Up @@ -60,16 +60,9 @@ function generateRequirementsFile(
pluginInstance
) {
const { serverless, servicePath, options, log } = pluginInstance;
if (
options.usePoetry &&
fse.existsSync(path.join(servicePath, 'pyproject.toml')) &&
isPoetryProject(servicePath)
) {
filterRequirementsFile(
path.join(servicePath, '.serverless/requirements.txt'),
targetFile,
pluginInstance
);
const modulePath = path.dirname(requirementsPath);
if (options.usePoetry && isPoetryProject(modulePath)) {
filterRequirementsFile(targetFile, targetFile, pluginInstance);
if (log) {
log.info(`Parsed requirements.txt from pyproject.toml in ${targetFile}`);
} else {
Expand Down Expand Up @@ -570,11 +563,7 @@ function copyVendors(vendorFolder, targetFolder, { serverless, log }) {
* @param {string} fileName
*/
function requirementsFileExists(servicePath, options, fileName) {
if (
options.usePoetry &&
fse.existsSync(path.join(servicePath, 'pyproject.toml')) &&
isPoetryProject(servicePath)
) {
if (options.usePoetry && isPoetryProject(path.dirname(fileName))) {
return true;
}

Expand Down Expand Up @@ -609,6 +598,8 @@ async function installRequirementsIfNeeded(
// Our source requirements, under our service path, and our module path (if specified)
const fileName = path.join(servicePath, modulePath, options.fileName);

await pyprojectTomlToRequirements(modulePath, pluginInstance);

// Skip requirements generation, if requirements file doesn't exist
if (!requirementsFileExists(servicePath, options, fileName)) {
return false;
Expand Down
33 changes: 17 additions & 16 deletions lib/poetry.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,25 @@ const tomlParse = require('@iarna/toml/parse-string');
/**
* poetry install
*/
async function pyprojectTomlToRequirements() {
if (!this.options.usePoetry || !isPoetryProject(this.servicePath)) {
async function pyprojectTomlToRequirements(modulePath, pluginInstance) {
const { serverless, servicePath, options, log, progress } = pluginInstance;

const moduleProjectPath = path.join(servicePath, modulePath);
if (!options.usePoetry || !isPoetryProject(moduleProjectPath)) {
return;
}

let generateRequirementsProgress;
if (this.progress && this.log) {
generateRequirementsProgress = this.progress.get(
if (progress && log) {
generateRequirementsProgress = progress.get(
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you validate how that behaves when there's multiple functions packaged individually at the same time? Are they being packaged in parallel or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I was not aware of this mechanism. I will try to investigate and get back to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I conducted an experiment with multiple functions packaged individually. It appears that this plugin is written to package the functions sequentially and does not support parallel packaging:

From installAllRequirements.js:

    for (const f of filteredFuncs) {
      if (!get(f, 'module')) {
        set(f, ['module'], '.');
      }

      // If we didn't already process a module (functions can re-use modules)
      if (!doneModules.includes(f.module)) {
        const reqsInstalledAt = await installRequirementsIfNeeded(
          f.module,
          f,
          this
        );
...

Note how the await installRequirementsIfNeeded is inside of the for loop. This will prevent parallel execution of the packaging of the functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for checking that 🙇

'python-generate-requirements-toml'
);
generateRequirementsProgress.update(
'Generating requirements.txt from "pyproject.toml"'
);
this.log.info('Generating requirements.txt from "pyproject.toml"');
log.info('Generating requirements.txt from "pyproject.toml"');
} else {
this.serverless.cli.log(
'Generating requirements.txt from pyproject.toml...'
);
serverless.cli.log('Generating requirements.txt from pyproject.toml...');
}

try {
Expand All @@ -42,15 +43,15 @@ async function pyprojectTomlToRequirements() {
'--with-credentials',
],
{
cwd: this.servicePath,
cwd: moduleProjectPath,
}
);
} catch (e) {
if (
e.stderrBuffer &&
e.stderrBuffer.toString().includes('command not found')
) {
throw new this.serverless.classes.Error(
throw new serverless.classes.Error(
`poetry not found! Install it according to the poetry docs.`,
'PYTHON_REQUIREMENTS_POETRY_NOT_FOUND'
);
Expand All @@ -59,16 +60,16 @@ async function pyprojectTomlToRequirements() {
}

const editableFlag = new RegExp(/^-e /gm);
const sourceRequirements = path.join(this.servicePath, 'requirements.txt');
const sourceRequirements = path.join(moduleProjectPath, 'requirements.txt');
const requirementsContents = fse.readFileSync(sourceRequirements, {
encoding: 'utf-8',
});

if (requirementsContents.match(editableFlag)) {
if (this.log) {
this.log.info('The generated file contains -e flags, removing them');
if (log) {
log.info('The generated file contains -e flags, removing them');
} else {
this.serverless.cli.log(
serverless.cli.log(
'The generated file contains -e flags, removing them...'
);
}
Expand All @@ -78,10 +79,10 @@ async function pyprojectTomlToRequirements() {
);
}

fse.ensureDirSync(path.join(this.servicePath, '.serverless'));
fse.ensureDirSync(path.join(servicePath, '.serverless'));
fse.moveSync(
sourceRequirements,
path.join(this.servicePath, '.serverless', 'requirements.txt'),
path.join(servicePath, '.serverless', modulePath, 'requirements.txt'),
{ overwrite: true }
);
} finally {
Expand Down
19 changes: 19 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,25 @@ test(
{ skip: !hasPython(3.6) }
);

test(
'poetry py3.6 can package flask with package individually option',
async (t) => {
process.chdir('tests/poetry_individually');
const path = npm(['pack', '../..']);
npm(['i', path]);

sls(['package'], { env: {} });
const zipfiles = await listZipFiles(
'.serverless/module1-sls-py-req-test-dev-hello.zip'
);
t.true(zipfiles.includes(`flask${sep}__init__.py`), 'flask is packaged');
t.true(zipfiles.includes(`bottle.py`), 'bottle is packaged');
t.true(zipfiles.includes(`boto3${sep}__init__.py`), 'boto3 is packaged');
t.end();
},
{ skip: !hasPython(3.6) }
);

test(
'py3.6 can package flask with package individually option',
async (t) => {
Expand Down
2 changes: 1 addition & 1 deletion tests/base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
"author": "",
"license": "ISC",
"dependencies": {
"serverless-python-requirements": "file:serverless-python-requirements-5.1.1.tgz"
"serverless-python-requirements": "file:serverless-python-requirements-5.3.1.tgz"
}
}
2 changes: 1 addition & 1 deletion tests/non_build_pyproject/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
"author": "",
"license": "ISC",
"dependencies": {
"serverless-python-requirements": "file:serverless-python-requirements-5.1.1.tgz"
"serverless-python-requirements": "file:serverless-python-requirements-5.3.1.tgz"
}
}
2 changes: 1 addition & 1 deletion tests/non_poetry_pyproject/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
"author": "",
"license": "ISC",
"dependencies": {
"serverless-python-requirements": "file:serverless-python-requirements-5.1.1.tgz"
"serverless-python-requirements": "file:serverless-python-requirements-5.3.1.tgz"
}
}
2 changes: 1 addition & 1 deletion tests/pipenv/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
"author": "",
"license": "ISC",
"dependencies": {
"serverless-python-requirements": "file:serverless-python-requirements-5.1.1.tgz"
"serverless-python-requirements": "file:serverless-python-requirements-5.3.1.tgz"
}
}
2 changes: 1 addition & 1 deletion tests/poetry/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@
"author": "",
"license": "ISC",
"dependencies": {
"serverless-python-requirements": "file:serverless-python-requirements-5.1.1.tgz"
"serverless-python-requirements": "file:serverless-python-requirements-5.3.1.tgz"
}
}
5 changes: 5 additions & 0 deletions tests/poetry_individually/module1/handler.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import requests


def hello(event, context):
return requests.get('https://httpbin.org/get').json()
17 changes: 17 additions & 0 deletions tests/poetry_individually/module1/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[tool.poetry]
name = "poetry"
version = "0.1.0"
description = ""
authors = ["Your Name <[email protected]>"]

[tool.poetry.dependencies]
python = "^3.6"
Flask = "^1.0"
bottle = {git = "https://[email protected]/bottlepy/bottle.git", tag = "0.12.16"}
boto3 = "^1.9"

[tool.poetry.dev-dependencies]

[build-system]
requires = ["poetry>=0.12"]
build-backend = "poetry.masonry.api"
14 changes: 14 additions & 0 deletions tests/poetry_individually/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "example",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "",
"license": "ISC",
"dependencies": {
"serverless-python-requirements": "file:serverless-python-requirements-5.3.1.tgz"
}
}
32 changes: 32 additions & 0 deletions tests/poetry_individually/serverless.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
service: sls-py-req-test

provider:
name: aws
runtime: python3.6

plugins:
- serverless-python-requirements
custom:
pythonRequirements:
zip: ${env:zip, self:custom.defaults.zip}
slim: ${env:slim, self:custom.defaults.slim}
slimPatterns: ${file(./slimPatterns.yml):slimPatterns, self:custom.defaults.slimPatterns}
slimPatternsAppendDefaults: ${env:slimPatternsAppendDefaults, self:custom.defaults.slimPatternsAppendDefaults}
dockerizePip: ${env:dockerizePip, self:custom.defaults.dockerizePip}
defaults:
zip: false
slimPatterns: false
slimPatternsAppendDefaults: true
slim: false
dockerizePip: false

package:
individually: true

functions:
hello:
handler: handler.hello
module: module1
package:
patterns:
- 'module1/**'