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

Running deploy_tools.py using pip on machines that use pip 2 by default causes invalid lambda function #188

Open
jmartini opened this issue Jan 23, 2019 · 4 comments

Comments

@jmartini
Copy link
Collaborator

When you install python3 via HomeBrew, you get a new executable python3 and a new pip, pip3. Our deploy tools call pip directly so it uses the old pip v2. This creates a lambda function with invalid python 2 versions of some of our dependencies.

@kami619
Copy link

kami619 commented Mar 1, 2019

this also happens on linux with python3 and pip3. even modifying the deploy_tools.sh with pip3 did not help. we would have to use the venv to get around this bug.

@AbdouSeck
Copy link

Hi everyone,

I was just looking around to see if there would be anything I could help with. This one issue caught my attention. The issue at hand has very much to do with the pip executable being invoked. Since it is being invoked using the run function from the subprocess standard library module, the call may not necessarily be to the pip executable associated with the python version running the script. If the pip command on your shell happens to have taken by a version different from the one running the deployment script, then you won't necessarily get the expected outcome.

The solution is to fix the pip installation arguments lines to the following:

    install_args = [
        sys.executable,
        "-m",
        "pip",
        "install",
        "-r",
        requirements_path,
        "-t",
        TEMP_DIR_PATH
    ]

    install_args_no_deps = [
        sys.executable,
        "-m",
        "pip",
        "install",
        "--no-deps",
        "-r",
        requirements_path_no_deps,
        "-t",
        TEMP_DIR_PATH
    ]

Adding sys.executable and "-m" at the beginning of the lists will make the installation commands start with /path/to/python_version -m pip. And, as long as your python version has an associated pip installation, the script should be able to install the dependencies properly.

Please note that the sys module will need to be added to the import section of the script.

Also, using a virtual environment may not necessarily solve the issue since subprocess.run will start a shell without caring much about whether or not you are in a virtual environment. Therefore, it will bypass the fact that the pip command in a virtual environment is tied to the interpreter in that activated environment.

In any case, I am happy to fork this repository, add the changes and submit a pull request. But I am wondering if @jmartini or @kami619 would want to take it from here?

Thanks,

Abdou

@jmartini
Copy link
Collaborator Author

@AbdouSeck sounds like a good solution to me. Go ahead and try it out and open a pull request!

@AbdouSeck
Copy link

@jmartini @kami619 The pull request #211 is up for review. One of the unit tests is failing due to a KeyError exception, but I am not particularly sure where that is coming. My branch is rebased onto the latest version of the master branch, so I am not quite sure where the error is coming from.
Please let me know what you think.

Thanks,

Abdou

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

No branches or pull requests

3 participants