Skip to content

Support noDeploy option when use layers #759

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
5 changes: 3 additions & 2 deletions lib/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const BbPromise = require('bluebird');
const fse = require('fs-extra');
const path = require('path');
const JSZip = require('jszip');
const { writeZip, addTree } = require('./zipTree');
const { writeZip, addTreeNoDeploy } = require('./zipTree');
const { sha256Path, getRequirementsLayerPath } = require('./shared');

BbPromise.promisifyAll(fse);
Expand Down Expand Up @@ -39,9 +39,10 @@ function zipRequirements() {
} else {
const rootZip = new JSZip();
const runtimepath = 'python';
const noDeploy = new Set(this.options.noDeploy || []);

promises.push(
addTree(rootZip.folder(runtimepath), src).then(() =>
addTreeNoDeploy(rootZip.folder(runtimepath), src, noDeploy).then(() =>
writeZip(rootZip, zipCachePath)
)
);
Expand Down
32 changes: 31 additions & 1 deletion lib/zipTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,36 @@ function addTree(zip, src) {
.then(() => zip); // Original zip for chaining.
}

/**
* Add a directory recursively to a zip file. Files in src will be added to the top folder of zip.
* @param {JSZip} zip a zip object in the folder you want to add files to.
* @param {string} src the source folder.
* @param {Object} noDeploy
* @return {Promise} a promise offering the original JSZip object.
*/
function addTreeNoDeploy(zip, src, noDeploy) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need a new function just for that as most of the logic is copied over from the original addTree. I think it would be better to just have the optional argument of noDeploy to the original function. What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the review :)
You are right. I'll fix it along with the test.

Copy link
Author

Choose a reason for hiding this comment

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

@pgrzesik I'm done adding test and fixing the code. Please check it out.

const srcN = path.normalize(src);

return fse
.readdirAsync(srcN)
.filter(name => !noDeploy.has(name))
.map((name) => {
const srcPath = path.join(srcN, name);

return fse.statAsync(srcPath).then((stat) => {
if (stat.isDirectory()) {
return addTreeNoDeploy(zip.folder(name), srcPath, noDeploy);
} else {
const opts = { date: stat.mtime, unixPermissions: stat.mode };
return fse
.readFileAsync(srcPath)
.then((data) => zip.file(name, data, opts));
}
});
})
.then(() => zip); // Original zip for chaining.
}

/**
* Write zip contents to a file.
* @param {JSZip} zip the zip object
Expand Down Expand Up @@ -81,4 +111,4 @@ function zipFile(zip, zipPath, bufferPromise, fileOpts) {
.then(() => zip);
}

module.exports = { addTree, writeZip, zipFile };
module.exports = { addTree, writeZip, zipFile , addTreeNoDeploy };