Skip to content

[v4.2.0] packaging fails with relative path requirements #245

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

Closed
Nihilight opened this issue Sep 16, 2018 · 11 comments · Fixed by #246
Closed

[v4.2.0] packaging fails with relative path requirements #245

Nihilight opened this issue Sep 16, 2018 · 11 comments · Fixed by #246
Labels

Comments

@Nihilight
Copy link

Nihilight commented Sep 16, 2018

Setup

OS: Linux Ubuntu 18.04 LTS
Working directory: ~/playground

pythonReqExample/
serverless/

./pythonReqExample:
foobar.py
__init__.py

./serverless:
package.json
pythonHandler/
requirements.txt
serverless.yml

./serverless/pythonHandler:
handler.py
__init__.py

serverless.yml:

service: relative-requirements

provider:
  name: aws
  region: us-west-2

functions:
  python36:
    runtime: python3.6
    memorySize: 512
    handler: pythonHandler/handler.handle
    package:
      include:
        - pythonHandler/*.py

plugins:
  - serverless-python-requirements

requirements.txt:

../pythonReqExample

Running DEBUG_SLS=* serverless package results in the following log:

~/playground/serverless$ SLS_DEBUG=* serverless package
Serverless: Load command config
Serverless: Load command config:credentials
Serverless: Load command create
Serverless: Load command install
Serverless: Load command package
Serverless: Load command deploy
Serverless: Load command deploy:function
Serverless: Load command deploy:list
Serverless: Load command deploy:list:functions
Serverless: Load command invoke
Serverless: Load command invoke:local
Serverless: Load command info
Serverless: Load command logs
Serverless: Load command login
Serverless: Load command logout
Serverless: Load command metrics
Serverless: Load command print
Serverless: Load command remove
Serverless: Load command rollback
Serverless: Load command rollback:function
Serverless: Load command slstats
Serverless: Load command plugin
Serverless: Load command plugin
Serverless: Load command plugin:install
Serverless: Load command plugin
Serverless: Load command plugin:uninstall
Serverless: Load command plugin
Serverless: Load command plugin:list
Serverless: Load command plugin
Serverless: Load command plugin:search
Serverless: Load command config
Serverless: Load command config:credentials
Serverless: Load command rollback
Serverless: Load command rollback:function
Serverless: Load command requirements
Serverless: Load command requirements:clean
Serverless: Load command requirements:install
Serverless: Load command requirements:cleanCache
Serverless: Invoke package
Serverless: Invoke aws:common:validate
Serverless: Invoke aws:common:cleanupTempDir
Serverless: Generated requirements from /home/playground/serverless/requirements.txt in /home/playground/serverless/.serverless/requirements.txt...
Serverless: Installing requirements from /home/playground/serverless/.serverless/requirements/requirements.txt ...
Invalid requirement: '../pythonReqExample'
It looks like a path. Does it exist ?

  Error --------------------------------------------------

  null

     For debugging logs, run again after setting the "SLS_DEBUG=*" environment variable.

  Stack Trace --------------------------------------------

Error: null
    at installRequirements (/home/playground/serverless/node_modules/serverless-python-requirements/lib/pip.js:259:11)
    at installRequirementsIfNeeded (/home/playground/serverless/node_modules/serverless-python-requirements/lib/pip.js:445:3)
    at ServerlessPythonRequirements.installAllRequirements (/home/playground/serverless/node_modules/serverless-python-requirements/lib/pip.js:524:29)
From previous event:
    at PluginManager.invoke (/home/.nvm/versions/node/v9.5.0/lib/node_modules/serverless/lib/classes/PluginManager.js:390:22)
    at PluginManager.run (/home/.nvm/versions/node/v9.5.0/lib/node_modules/serverless/lib/classes/PluginManager.js:421:17)
    at variables.populateService.then.then (/home/.nvm/versions/node/v9.5.0/lib/node_modules/serverless/lib/Serverless.js:157:33)
    at runCallback (timers.js:756:18)
    at tryOnImmediate (timers.js:717:5)
    at processImmediate [as _immediateCallback] (timers.js:697:5)
From previous event:
    at Serverless.run (/home/.nvm/versions/node/v9.5.0/lib/node_modules/serverless/lib/Serverless.js:144:8)
    at serverless.init.then (/home/.nvm/versions/node/v9.5.0/lib/node_modules/serverless/bin/serverless:43:50)
    at <anonymous>

  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com

  Your Environment Information -----------------------------
     OS:                     linux
     Node Version:           9.5.0
     Serverless Version:     1.29.2

Let me know if you need any more info.
Thanks!

@AndrewFarley
Copy link
Contributor

I've got you, fix incoming...

@dschep
Copy link
Contributor

dschep commented Sep 16, 2018

Thanks @AndrewFarley. I was just starting on this too. Removing targetFolder as the cwd in the spawn arguments seemed to do the trick for me. Haven't run the full test suite yet, so I'll just let you finish up you fix 😄

@dschep
Copy link
Contributor

dschep commented Sep 16, 2018

But I think the fix is only for non-docker. I don't think there's a reasonable way to make a path like that work with docker.

@AndrewFarley
Copy link
Contributor

@dschep Want to have a brief conversation? I already did a fix which is totally different... inside generate requirements I auto-prepended two more cd backwards. And I was going to add a "error" if you do this with docker enabled, thoughts?

  // Fix any items with ../ in them...
  for (var i=filteredRequirements.length; i--;) {
    if (filteredRequirements[i].startsWith('../')) {
      filteredRequirements[i] = '../../' + filteredRequirements[i];
    }
  }

@dschep
Copy link
Contributor

dschep commented Sep 16, 2018

Ah. yeah, I think I prefer my fix. There's no reason to run with the target as the working directory AFAICT. Tests seem to be passing with my change. This is the change:

diff --git a/lib/pip.js b/lib/pip.js
index 044f6c7..39d52ab 100644
--- a/lib/pip.js
+++ b/lib/pip.js
@@ -227,7 +227,7 @@ function installRequirements(targetFolder, serverless, options) {
     const preparedPath = dockerPathForWin(options, targetFolder);
     cmdOptions.push(getStripCommand(options, preparedPath));
   }
-  let spawnArgs = { cwd: targetFolder, shell: true };
+  let spawnArgs = { shell: true };
   if (process.env.SLS_DEBUG) {
     spawnArgs.stdio = 'inherit';
   }

@AndrewFarley
Copy link
Contributor

Yeah, I like your change a lot more. Let me try all tests with your change, and then throw a error incase this is present with docker enabled, eh? :)

@dschep
Copy link
Contributor

dschep commented Sep 16, 2018

Cool, yeah I was thinking we could still incorporate your idea of throwing an error if you find a requirement starting with ../ or / and docker is enabled.

dschep added a commit that referenced this issue Sep 16, 2018
This fixes #245 by making it so that relative paths in dependencies
resolve correctly
@Nihilight
Copy link
Author

Hey @dschep @AndrewFarley
Any estimation on when this is going to be merged and published?

@dschep
Copy link
Contributor

dschep commented Sep 17, 2018

I can merge it anytime, I was waiting for @AndrewFarley to push a few of his changes, but they aren't necessary for it to work. For now you can also just install the plugin from #246

npm i github:unitedincome/serverless-python-requirements#rel-dep

@AndrewFarley
Copy link
Contributor

AndrewFarley commented Sep 17, 2018 via email

dschep added a commit that referenced this issue Sep 18, 2018
This fixes #245 by making it so that relative paths in dependencies
resolve correctly
@dschep
Copy link
Contributor

dschep commented Sep 18, 2018

released in v4.2.4

@dschep dschep added the bug label Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants