Fix Windows support for dockerizePip #116

heri16 · 2017-12-18T15:50:16Z

Changed timeout to 10 seconds just in case some automated build process have a hard time contacting its docker server/backend engine.

heri16 · 2017-12-18T15:54:31Z

Decided against using regExp positive lookahead expression as some are not familiar with it. Would a lookahead expression be clearer?

this.servicePath.replace(/\\(?=[^\s])/g, '/');

Probably not, since I'm not even 100% sure what the code is trying to attempt now. Is this correct?

if process.platform indicates windows:

replace all \ with / (and something whitespace, but I'm not sure what)

if docker platform is windows:

convert /foo/bar/baz to foo:/bar/baz

else-if WSL:

strip /mnt from start of string.

if docker platform is windows:

convert /foo/bar/baz to foo:/bar/baz

I think the following might cleaner/clearer:

let bindPath = this.servicePath; const cliPlatform = ps.stdout.trim(); if (process.platform === 'win32') { bindPath = bindPath.replace(/\\([^\s])/g, '/$1'); } else if (isWsl) { bindPath = bindPath.replace(/^\/mnt\//, '/'); } if (cliPlatform === 'windows') { bindPath = bindPath.replace(/^\/(\w)\//i, '$1:/'); }

nickweavers · 2017-12-18T14:21:57Z

Looks like this should work in either the case where we have /mn t/c (default) or where users have linked /mnt/c to /c (as in my case). I'd be happy to try it.

Awesome, I'd love any extra Windows testing seeing as I can't do it myself. You can test this PR before it's merged by installing with npm i heri16/serverless-python-requirements#patch-3

Speaking of Windows testing, @heri16 or @nickweavers do either of you know if AppVeyor has useful[0] docker support? I'll probably set it up anyway to test windows support without docker, but it'd be great to test the docker options too.

[0] I say useful, bc while CircleCI has docker support, it cant -v mount volumes, making it useless for this plugin's docker features

Sorry @dschep, I can't help with AppVeyor - haven't tried it.

Thanks for the instructions on how to install from the branch before it's merge... that's really useful. Here what happened when I tried it:

nickw@DESKTOP-OREBB4V:/i/ddfs-serverless/python-s3-thumbnail $npm i heri16/serverless-python-requirements#patch-3 npm WARN [email protected] No repository field. npm WARN [email protected] No license field. + [email protected] added 1 package and updated 1 package in 3.556s

$sls deploy -v Serverless: Installing required Python packages with python2.7... Serverless: Docker Image: lambci/lambda:build-python2.7 Reference Error ---------------------------------------- isWsl is not defined For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable. Get Support -------------------------------------------- Docs: docs.serverless.com Bugs: github.com/serverless/serverless/issues Forums: forum.serverless.com Chat: gitter.im/serverless/serverless Your Environment Information ----------------------------- OS: linux Node Version: 9.3.0 Serverless Version: 1.24.1

Maybe not having the test on the require('is-wsl') is better since it then fails with the "not defined" message rather that what was being done in the catch, which I think was to set isWsl to false making it appear we aren't under wsl?

Hmm. Interesting. As I mentioned to @heri16, I'm not sure the try/catch is the right way to go (the dependency isn't conditional and it should run on all platforms.) But if it is necessary, I think this would avoid the above issue:

let isWsl = false try { isWsl = require('is-wsl'); } catch (e) {}

Exactly the same result I'm afraid:

nickw@DESKTOP-OREBB4V:/i/ddfs-serverless/python-s3-thumbnail $npm i heri16/serverless-python-requirements#patch-3 npm WARN [email protected] No repository field. npm WARN [email protected] No license field. + [email protected] updated 1 package in 1.032s nickw@DESKTOP-OREBB4V:/i/ddfs-serverless/python-s3-thumbnail $sls deploy -v Serverless: Installing required Python packages with python2.7... Serverless: Docker Image: lambci/lambda:build-python2.7 Reference Error ---------------------------------------- isWsl is not defined For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable. Get Support -------------------------------------------- Docs: docs.serverless.com Bugs: github.com/serverless/serverless/issues Forums: forum.serverless.com Chat: gitter.im/serverless/serverless Your Environment Information ----------------------------- OS: linux Node Version: 9.3.0 Serverless Version: 1.24.1

Sorry for the confusion @nickweavers this branch hasn't been updated yet, so that's expected 😜

Hi @nickweavers, apologies for the mixup. Do try again with npm i heri16/serverless-python-requirements#patch-3

Sorry for the delay, looks like the isWsl patch worked and this thing takes a while to run now! Thanks for your efforts guys, really appreciated! There's still an error, but it looks like a user error and not a system/scripting error 👍

nickw@DESKTOP-OREBB4V:/i/ddfs-serverless/python-s3-thumbnail $sls deploy -v Serverless: Installing required Python packages with python2.7... Serverless: Docker Image: lambci/lambda:build-python2.7 Serverless: Linking required Python packages... Serverless: Packaging service... Serverless: Excluding development dependencies... Serverless: Unlinking required Python packages... Serverless: Creating Stack... Serverless: Checking Stack create progress... CloudFormation - CREATE_IN_PROGRESS - AWS::CloudFormation::Stack - python-s3-thumbnail-dev CloudFormation - CREATE_IN_PROGRESS - AWS::S3::Bucket - ServerlessDeploymentBucket CloudFormation - CREATE_IN_PROGRESS - AWS::S3::Bucket - ServerlessDeploymentBucket CloudFormation - CREATE_COMPLETE - AWS::S3::Bucket - ServerlessDeploymentBucket CloudFormation - CREATE_COMPLETE - AWS::CloudFormation::Stack - python-s3-thumbnail-dev Serverless: Stack create finished... Serverless: Uploading CloudFormation file to S3... Serverless: Uploading artifacts... Serverless: Uploading service .zip file to S3 (12.7 MB)... Serverless: Validating template... Serverless: Updating Stack... Serverless: Checking Stack update progress... CloudFormation - UPDATE_IN_PROGRESS - AWS::CloudFormation::Stack - python-s3-thumbnail-dev CloudFormation - CREATE_IN_PROGRESS - AWS::IAM::Role - IamRoleLambdaExecution CloudFormation - CREATE_IN_PROGRESS - AWS::Logs::LogGroup - S3DashthumbnailDashgeneratorLogGroup CloudFormation - CREATE_IN_PROGRESS - AWS::IAM::Role - IamRoleLambdaExecution CloudFormation - CREATE_IN_PROGRESS - AWS::Logs::LogGroup - S3DashthumbnailDashgeneratorLogGroup CloudFormation - CREATE_COMPLETE - AWS::Logs::LogGroup - S3DashthumbnailDashgeneratorLogGroup CloudFormation - CREATE_COMPLETE - AWS::IAM::Role - IamRoleLambdaExecution CloudFormation - CREATE_IN_PROGRESS - AWS::Lambda::Function - S3DashthumbnailDashgeneratorLambdaFunction CloudFormation - CREATE_IN_PROGRESS - AWS::Lambda::Function - S3DashthumbnailDashgeneratorLambdaFunction CloudFormation - CREATE_COMPLETE - AWS::Lambda::Function - S3DashthumbnailDashgeneratorLambdaFunction CloudFormation - CREATE_IN_PROGRESS - AWS::Lambda::Version - S3DashthumbnailDashgeneratorLambdaVersionic9c2p8ZprSApdQzriL3OpL0Rl4pkbuT1TUITtMnrI CloudFormation - CREATE_IN_PROGRESS - AWS::Lambda::Permission - S3DashthumbnailDashgeneratorLambdaPermissionStephanes3thumbnailgeneratorS3 CloudFormation - CREATE_IN_PROGRESS - AWS::Lambda::Version - S3DashthumbnailDashgeneratorLambdaVersionic9c2p8ZprSApdQzriL3OpL0Rl4pkbuT1TUITtMnrI CloudFormation - CREATE_IN_PROGRESS - AWS::Lambda::Permission - S3DashthumbnailDashgeneratorLambdaPermissionStephanes3thumbnailgeneratorS3 CloudFormation - CREATE_COMPLETE - AWS::Lambda::Version - S3DashthumbnailDashgeneratorLambdaVersionic9c2p8ZprSApdQzriL3OpL0Rl4pkbuT1TUITtMnrI CloudFormation - CREATE_COMPLETE - AWS::Lambda::Permission - S3DashthumbnailDashgeneratorLambdaPermissionStephanes3thumbnailgeneratorS3 CloudFormation - CREATE_IN_PROGRESS - AWS::S3::Bucket - S3BucketStephanes3thumbnailgenerator CloudFormation - CREATE_FAILED - AWS::S3::Bucket - S3BucketStephanes3thumbnailgenerator CloudFormation - UPDATE_ROLLBACK_IN_PROGRESS - AWS::CloudFormation::Stack - python-s3-thumbnail-dev CloudFormation - UPDATE_ROLLBACK_COMPLETE_CLEANUP_IN_PROGRESS - AWS::CloudFormation::Stack - python-s3-thumbnail-dev CloudFormation - DELETE_SKIPPED - AWS::Lambda::Version - S3DashthumbnailDashgeneratorLambdaVersionic9c2p8ZprSApdQzriL3OpL0Rl4pkbuT1TUITtMnrI CloudFormation - DELETE_COMPLETE - AWS::S3::Bucket - S3BucketStephanes3thumbnailgenerator CloudFormation - DELETE_IN_PROGRESS - AWS::Lambda::Permission - S3DashthumbnailDashgeneratorLambdaPermissionStephanes3thumbnailgeneratorS3 CloudFormation - DELETE_COMPLETE - AWS::Lambda::Permission - S3DashthumbnailDashgeneratorLambdaPermissionStephanes3thumbnailgeneratorS3 CloudFormation - DELETE_IN_PROGRESS - AWS::Lambda::Function - S3DashthumbnailDashgeneratorLambdaFunction CloudFormation - DELETE_COMPLETE - AWS::Lambda::Function - S3DashthumbnailDashgeneratorLambdaFunction CloudFormation - DELETE_IN_PROGRESS - AWS::Logs::LogGroup - S3DashthumbnailDashgeneratorLogGroup CloudFormation - DELETE_IN_PROGRESS - AWS::IAM::Role - IamRoleLambdaExecution CloudFormation - DELETE_COMPLETE - AWS::Logs::LogGroup - S3DashthumbnailDashgeneratorLogGroup CloudFormation - DELETE_COMPLETE - AWS::IAM::Role - IamRoleLambdaExecution CloudFormation - UPDATE_ROLLBACK_COMPLETE - AWS::CloudFormation::Stack - python-s3-thumbnail-dev Serverless: Operation failed! Serverless Error --------------------------------------- An error occurred: S3BucketStephanes3thumbnailgenerator - stephane-s3-thumbnail-generator already exists. Get Support -------------------------------------------- Docs: docs.serverless.com Bugs: github.com/serverless/serverless/issues Forums: forum.serverless.com Chat: gitter.im/serverless/serverless Your Environment Information ----------------------------- OS: linux Node Version: 9.3.0 Serverless Version: 1.24.1

I'll try deleting the bucket and rerunning, and confirm if it's all good when I've done that.

-Original file line number
+Diff line change
@@ -1,6 +1,7 @@
     const fse = require('fs-extra');
     const path = require('path');
     const {spawnSync} = require('child_process');
+    const isWsl = require('is-wsl');
     /**
      * pip install the requirements to the .serverless/requirements directory
@@ Expand Down Expand Up / @@ -50,29 +51,39 @@ function installRequirements() { @@
         this.serverless.cli.log(`Docker Image: ${this.options.dockerImage}`);
-        // Check docker server os type from 'docker version'
-        let volPath;
+        // Determine os platform of docker CLI from 'docker version'
         options = [
-          'version', '--format', '{{with .Server}}{{.Os}}{{end}}'
+          'version', '--format', '{{with .Client}}{{.Os}}{{end}}'
         ];
-        const ps = spawnSync(cmd, options, {'timeout': 2000, 'encoding': 'utf-8'});
+        const ps = spawnSync(cmd, options, {'timeout': 10000, 'encoding': 'utf-8'});
         if (ps.error) {
           if (ps.error.code === 'ENOENT') {
             throw new Error('docker not found! Please install it.');
           }
           throw new Error(ps.error);
-        }
-        if (ps.status !== 0) {
+        } else if (ps.status !== 0) {
           throw new Error(ps.stderr);
-        } else if (ps.stdout.trim() === 'windows') {
-          volPath = this.servicePath.replace(/\\/g, '/').replace(/^\/mnt\//, '/');
+        }
+        let bindPath;
+        const cliPlatform = ps.stdout.trim();
+        if (process.platform === 'win32') {
+          bindPath = this.servicePath.replace(/\\([^\s])/g, '/$1');
+          if (cliPlatform === 'windows') {
+            bindPath = bindPath.replace(/^\/(\w)\//i, '$1:/');
+          }
+        } else if (isWsl) {
+          bindPath = this.servicePath.replace(/^\/mnt\//, '/');
+          if (cliPlatform === 'windows') {
+            bindPath = bindPath.replace(/^\/(\w)\//i, '$1:/');
+          }
         } else {
-          volPath = this.servicePath;
+          bindPath = this.servicePath;
         }
         options = [
           'run', '--rm',
-          '-v', `${volPath}:/var/task:z`,
+          '-v', `${bindPath}:/var/task:z`,
         ];
         if (process.platform === 'linux')
           options.push('-u', `${process.getuid()}:${process.getgid()}`);
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -51,6 +51,7 @@ @@
         "fs-extra": "^3.0.1",
         "glob-all": "^3.1.0",
         "graceful-fs": "^4.1.11",
+        "is-wsl": "^1.1.0",
         "lodash": "^4.13.1"
       }
     }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix Windows support for dockerizePip #116

Diff view

Diff view

There are no files selected for viewing

heri16 Dec 18, 2017

Uh oh!

heri16 Dec 18, 2017

Uh oh!

dschep Dec 18, 2017 •

edited

Loading

Uh oh!

heri16 Jun 15, 2018

Uh oh!

nickweavers Dec 18, 2017

Uh oh!

dschep Dec 18, 2017

Uh oh!

dschep Dec 18, 2017 •

edited

Loading

Uh oh!

nickweavers Dec 18, 2017 •

edited

Loading

Uh oh!

dschep Dec 18, 2017 •

edited

Loading

Uh oh!

nickweavers Dec 18, 2017

Uh oh!

dschep Dec 18, 2017

Uh oh!

heri16 Dec 18, 2017

Uh oh!

nickweavers Dec 18, 2017

Uh oh!

dschep Dec 18, 2017

Uh oh!

Fix Windows support for dockerizePip #116

Fix Windows support for dockerizePip #116

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschep Dec 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschep Dec 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickweavers Dec 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschep Dec 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dschep Dec 18, 2017 •

edited

Loading

dschep Dec 18, 2017 •

edited

Loading

nickweavers Dec 18, 2017 •

edited

Loading

dschep Dec 18, 2017 •

edited

Loading