Skip to content

Support serverless.yml and serverless.yaml in getBindPath() #213

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.

Sign up for GitHub

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 8 commits into from
Jul 16, 2018
Merged

Support serverless.yml and serverless.yaml in getBindPath() #213

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
59566e4
Support serverless.yml and serverless.yaml
kichik Jul 3, 2018
d221b7e
Restore comment
kichik Jul 3, 2018
5d8a9a1
Support serverless.json
kichik Jul 10, 2018
9750262
Merge branch 'master' into patch-4
kichik Jul 10, 2018
6101797
use this.serverless.cli.log
kichik Jul 13, 2018
8e5ad85
prettier
kichik Jul 13, 2018
1c72e9a
simpler way to use serverless.cli.log
kichik Jul 14, 2018
e012cd0
prettier
kichik Jul 14, 2018
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
34 changes: 30 additions & 4 deletions lib/docker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const { spawnSync } = require('child_process');
const isWsl = require('is-wsl');
const fse = require('fs-extra');
const path = require('path');

/**
* Helper function to run a docker command
Expand Down Expand Up @@ -32,24 +34,46 @@ function buildImage(dockerFile) {
return imageName;
}

/**
* Find a file that exists on all projects so we can test if Docker can see it too
* @param {string} servicePath
* @return {string} file name
*/
function findTestFile(servicePath) {
if (fse.pathExistsSync(path.join(servicePath, 'serverless.yml'))) {
return 'serverless.yml';
}
if (fse.pathExistsSync(path.join(servicePath, 'serverless.yaml'))) {
return 'serverless.yaml';
Copy link
Contributor

Choose a reason for hiding this comment

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

Did serverless drop support for serverless.json?

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 was not aware that's an option. Let me add that.

}
if (fse.pathExistsSync(path.join(servicePath, 'serverless.json'))) {
return 'serverless.json';
}
throw new Error('Unable to find serverless.yml or serverless.yaml or serverless.json for getBindPath()');
}

/**
* Test bind path to make sure it's working
* @param {string} bindPath
* @return {boolean}
*/
function tryBindPath(bindPath) {
function tryBindPath(bindPath, testFile) {
const options = [
'run',
'--rm',
'-v',
`${bindPath}:/test`,
'alpine',
'ls',
'/test/serverless.yml'
`/test/${testFile}`
];
try {
const ps = dockerCommand(options);
return ps.stdout.trim() === '/test/serverless.yml';
if (process.env.SLS_DEBUG) {
console.log(`Trying bindPath ${bindPath} (${options})`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use the serverless logging facility instead of console.log?

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 can get to it later this week. We should consider passing this as the second argument to .map() in installAllRequirements() to make this simpler and so we don't have to pass this.serverless around.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that idea! Get to it when you get to it 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It ended up being a bigger surgery than I expected as we don't always call these functions with .map() and so this in that case is the global context. We might have to create a class to make this make sense there.

console.log(ps.stdout.trim());
}
return ps.stdout.trim() === `/test/${testFile}`;
} catch (err) {
return false;
}
Expand Down Expand Up @@ -99,10 +123,12 @@ function getBindPath(servicePath) {
bindPaths.push(`/mnt/${drive.toLowerCase()}/${path}`);
bindPaths.push(`/mnt/${drive.toUpperCase()}/${path}`);
bindPaths.push(`${drive.toUpperCase()}:/${path}`);

const testFile = findTestFile(servicePath);

for (let i = 0; i < bindPaths.length; i++) {
const bindPath = bindPaths[i];
if (tryBindPath(bindPath)) {
if (tryBindPath(bindPath, testFile)) {
return bindPath;
}
}
Expand Down