Skip to content
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

Fix eb not found error when installing via ebcli_installer.py #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

surajrav
Copy link

@surajrav surajrav commented Mar 4, 2020

Ensure that subprocess.Popen calls happen in the activated virtualenv.

Without this the awsebcli python module was getting installed to
the location pointed out by sys.executable which in this scenario,
wherein the virtual env is activated via an exec/execfile of the
activate_this.py code, is not set to that of the virtualenv python
executable and hence the base system's pip kicks in and installs
it in the base system's python site-packages path. This further
causes an issue when the eb executable wrapper tries to reach it
at the virtualenv's bin_location/eb and results in an eb not
found error.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Without this the `awsebcli` python module was getting installed to
the location pointed out by `sys.executable` which in this scenario,
wherein the virtual env is activated via an exec/execfile of the
`activate_this.py` code, is not set to that of the virtualenv python
executable and hence the base system's `pip` kicks in and installs
it in the base system's python site-packages path. This further
causes an issue when the eb executable wrapper tries to reach it
at the virtualenv's `bin_location/eb` and results in an `eb` not
found error.
@rahulrajaram
Copy link
Contributor

@surajrav , thanks for reaching out with your PR. I did not realize that passing shell=True actually does not pass the parent process' environment variables to the spawned shell. Without the shell=True argument, the environment variables of the parent will be inherited by the child.

My initial thoughts are that:

  1. I think this will fix a few of our existing issues
  2. we should pass all of the variables in os.environ, ideally to mimic the situation when shell=True is not passed. Alternately -- and I don't remember why we decided to pass shell=True in the first place -- we should look into removing shell=True. This would solve the ensure that the pip inside the virtualenv dir will be be invoked.
  3. We don't need the change to the eb.sh file-string that will be used to generate the eb wrapper on the fly.

I need to test your code changes. I also need to test the alternative I've proposed in 2.. I'll get back to you shortly.

@surajrav
Copy link
Author

surajrav commented Mar 4, 2020

Hi @rahulrajaram, thanks for the quick response.

I agree with your analysis of this, except for point number 3:
It failed for me without patching the wrapper since I hadn't made it initially and it still failed and the reason for that is that, post getting to the eb executable it will execute:

#!/Users/sravichandran/.ebcli-virtual-env/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from ebcli.core.ebcore import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

which at said point failed to import from ebcli.core.ebcore import main

Having said that the above snippet does have the #!/Users/sravichandran/.ebcli-virtual-env/bin/python line so I get your suspicion on the fact that it is not needed but this is what I observed. I think the import system relies on not just the shebang but also the sys.path, but I haven't vetted that theory.

Hope this helps.

And if I can help in any other way

@rahulrajaram
Copy link
Contributor

I agree with your analysis of this, except for point number 3:

Yes, the reason that you don't need to modify the text for the eb wrapper is that the correct virtualenv will be activated by the script here, and because, without shell=True, subprocess.Popen will actually pass the parent's environment variables to the child, passing the env. vars in this case is redundant. Your experience suggests otherwise. Can you confirm once again? Either way, I'll test the case, on which note -- can you please provide STR to reproduce this?

@surajrav
Copy link
Author

surajrav commented Mar 9, 2020

For reproduction here is what I did:

  1. Git clone this repo
  2. follow the second option from https://github.com/aws/aws-elastic-beanstalk-cli-setup#3-advanced-use:
python scripts/ebcli_installer.py --python-installation /path/to/some/python/on/your/computer

I try it again post nuking the ~/.ebcli-virtual-env/ and nuking the shell=True from the pip install subprocess and report back if that makes this work or not. I may not be able to get to it tmrw though.

@rahulrajaram
Copy link
Contributor

Thanks for the STR.

@surajrav
Copy link
Author

ping ^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants