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 issues due to pip 10 release #14

Merged
merged 3 commits into from
May 1, 2018
Merged

Fix issues due to pip 10 release #14

merged 3 commits into from
May 1, 2018

Conversation

paulbovbel
Copy link
Member

@paulbovbel paulbovbel commented May 1, 2018

Lock down pip version at 9.0.3

@@ -71,8 +75,8 @@ def __init__(self,

self.preinstall = preinstall
self.upgrade_pip = upgrade_pip
self.pip_version = pip_version

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have a # (pbovbel): allow pinning the pip version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


# (pbovbel): make logging optional, since --log overrides -q flag for pip
if log_file:
self.log_file = log_file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assign self.log_file=None when log_file is false-y, to avoid having the exposed attributes change based on its __init__ arguments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

# Add in any user supplied pip args
self.pip_args.extend(extra_pip_arg)

# (pbovbel) Update pip_upgrade_args to keep flags like -q

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing : here and below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded

# First, bootstrap pip with a reduced option set (well-supported options)
print(self.pip_preinstall_prefix + self.pip_upgrade_args + ['-U', 'pip'])
subprocess.check_call(self.pip_preinstall_prefix + self.pip_upgrade_args + ['-U', 'pip'])
print(self.pip_preinstall_prefix + self.pip_upgrade_args + ['-U', pip_package])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Easier to keep these strings in sync if you assigned it to a variable and used it for both, but I recognize that means diverging from upstream.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the concern

@paulbovbel paulbovbel merged commit 8e9eb00 into devel May 1, 2018
@paulbovbel paulbovbel deleted the fix-pip branch March 18, 2019 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants