Fix Windows support for dockerizePip #116
Conversation
| bindPath = bindPath.replace(/^\/(\w)\//i, '$1:/'); | ||
| } | ||
| } else if (isWsl) { | ||
| bindPath = this.servicePath.replace(/^\/mnt\//, '/'); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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) {}There was a problem hiding this comment.
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
There was a problem hiding this comment.
Sorry for the confusion @nickweavers this branch hasn't been updated yet, so that's expected 😜
There was a problem hiding this comment.
Hi @nickweavers, apologies for the mixup. Do try again with npm i heri16/serverless-python-requirements#patch-3
There was a problem hiding this comment.
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.
lib/pip.js
Outdated
| // Detection of Windows Subsystem for Linux | ||
| try { | ||
| const isWsl = require('is-wsl'); | ||
| } catch (e) { |
There was a problem hiding this comment.
Is there a good reason to expect the is-wsl module not to be importable? It's in the dependencies and doesn't look like it'd crash on other platforms for any reason (and I just tested it on linux without import errors).
|
Yeah... That was my bad. Was multitasking while simultaneously debugging some python3 code. I think isWsl is not expected to fail as Linux platforms should always have the `/proc` according to Unix standards (I think).
That was some horrible scoping mistake. Thank goodness we have peer code review.
@dschep one is the right way to go.
Amended my commit.
|
heri16
force-pushed
the
patch-3
branch
2 times, most recently
from
ce8eb24 to
a24cbd1
Compare
| ]; | ||
| const ps = spawnSync(cmd, options, {'timeout': 2000, 'encoding': 'utf-8'}); | ||
| const ps = spawnSync(cmd, options, {'timeout': 10000, 'encoding': 'utf-8'}); |
There was a problem hiding this comment.
Changed timeout to 10 seconds just in case some automated build process have a hard time contacting its docker server/backend engine.
| const cliPlatform = ps.stdout.trim(); | ||
| if (process.platform === 'win32') { | ||
| bindPath = this.servicePath.replace(/\\([^\s])/g, '/$1'); | ||
| if (cliPlatform === 'windows') { |
There was a problem hiding this comment.
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, '/');
There was a problem hiding this comment.
Probably not, since I'm not even 100% sure what the code is trying to attempt now. Is this correct?
- if
process.platformindicates windows:- replace all
\with/(and something whitespace, but I'm not sure what) - if docker platform is windows:
- convert
/foo/bar/baztofoo:/bar/baz
- convert
- replace all
- else-if WSL:
- strip
/mntfrom start of string. - if docker platform is windows:
- convert
/foo/bar/baztofoo:/bar/baz
- convert
- strip
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:/');
}
|
There are two ways of using Docker for Windows from Windows Subsystem for Linux (WSL):
There are also other ways of using Docker for Windows (if you don't want to use, or don't care about WSL):
With or without WSL, the drive letter must be activated within the Windows Docker Taskbar - Shared Drive Settings, as Docker for Windows (as of this writing) is still running linux containers within an ArchLinux VM, just like Docker for Mac. Of course WSL is useful if your cloud service provider is AWS Lambda (or alike) as it helps to ensure that your python code would have at least no problems running on Linux, the Operating System used to host many serverless platforms, before actual code deployment. |
Right.. not TOO much reason to use Docker support if using WSL. Folks might want to use the Feel free to add any notes about docker/windows/wsl to the Windows&Docker section of the readme to this PR, @heri16 |
So, forgive me if I haven't fully grasped the concepts as I'm just a beginner with Serverless, but in the tutorial I was working through we were creating a thumbnail generator that needed to monitor an S3 bucket for .png image files being uploaded to it, then firing events to trigger our Lambda function to generate thumbnails for them. For this we had dependencies for the image manipulation libraries so as I understood it, this is where serverless-python-requirements stepped in to help with bundling our requirements together into a container atop the aws lambda base (which I guess is Linux). |
|
Also, from what I'm seeing in the forums over the last couple of years, WSL was understandably viewed with cynicism initially, but is now garnering quite a following among those who have seen big strides made in it being able to do some development jobs very adequately (web development using Lamp stack and Git running on it , and devs being able to use Windows based IDE's from Windows to work on the code files). It looks like the Windows team responsible for WSL have plenty of enthusiasm to go after the harder problems to make it even more useful too, so there's a good chance it will become more ubiquitous :) |
Yes and no. You need Docker if your deps are compiled (the yes) and if you're not on a system who's architecture matches lambda (the no). So if you're on 64bit linux (which WSL is AFAIK), you don't need Docker, unlike if you're on Windows (without WSL) or macOS, thus the I personally don't dockerize any installs with this plugin because I use 64bit Linux on the desktop, but my colleagues use macOS so I build the dockerize support in this plugin (and the |
|
Thanks for the insightful info @dschep. I just got the deploy to complete by deleting my CloudFormation stack, disabling the plugin and rerunning it. The deploy runs in much less time. I then tried again with the plugin enabled: Again, all good so both paths working under wsl for me 👍 |
|
Glad to hear it works! I'll merge this soon and make a new release. |
|
Running serverless on 64-bit Linux does not always guarantee that it will run on AWS Lambda due to ABI-Compatibility issues between the different Linux distributions. (Ubuntu binaries and packages can't just be copied into Fedora or Suse or ArchLinux to be executed and vice versa.) There are certainly exceptions to this like python manywheels, of which I still don't quite understand the intricacies yet. I would assume it avoids using any shared libraries by doing static compilation. I would assume not all python libraries actually support manywheels. The whole purpose of docker is to containerize the whole ABI-Environment, so that you can run applications written for any other distribution of Linux. Therefore my current understanding is that unless you are running a Redhat derivative of Linux (which coincidentally happens to similar to Amazon Linux), it is not safe to not enable dockerizePip. We must not forget there are othrr cloud platforms supported by Serverless Framework like Microsoft Azure and Google Cloud Platform which might not be using a Redhat-derivative for the Lambda equivalent. |
|
Most people use Ubuntu or SuseLinux on WSL, as they are the only distribution supported by Microsoft (as of this writing). |
|
Good point @heri16, and one of the attractions of Serverless to me is having the potential to switch to a different cloud platform depending on circumstances. |
|
Correct @heri16, and manylinux wheels do in fact work for many cases even on the wrong Linux, I do it every day. This is because nothing is being compiled, the manylinux wheel is precompiled for.. wait for it... many linuxes! 😀 . ABI compatibility is more necessary when actually compiling yourself. |
Reattempt of #110 due to WSL complexity