Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Don't default index url to public pypi. Defer to underlying libs #52

Merged
merged 3 commits into from Mar 29, 2021

Conversation

analogue
Copy link
Contributor

@analogue analogue commented Mar 29, 2021

Testing done: `tox -e upgrade-requirements' now works with serviecs/ad_realtime_bidding

Internal ticket: CORESERV-10904

@analogue analogue requested a review from ymilki March 29, 2021 16:18
@analogue analogue changed the title Don't default index url to publib pypi. Defer to underlying libs Don't default index url to public pypi. Defer to underlying libs Mar 29, 2021
Copy link
Member

@ymilki ymilki left a comment

Choose a reason for hiding this comment

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

lgtm. We want to rely on the normal expected defaults for pypi.

Copy link
Contributor

@asottile asottile left a comment

Choose a reason for hiding this comment

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

please don't change this -- it breaks my uses of this

the service in question should be fixed instead to utilize the --index-url argument explicitly

@analogue
Copy link
Contributor Author

analogue commented Mar 29, 2021

please don't change this -- it breaks my uses of this

the service in question should be fixed instead to utilize the --index-url argument explicitly

We'd like the default to be picked up from /etc/pip.conf which I believe is the default for the rest of python / pip related tools. Having upgrade-requirements as the the outlier is not desirable.

If you consider this a breaking change, I can bump the version accordingly.

@analogue analogue merged commit 8f7a138 into master Mar 29, 2021
@analogue analogue deleted the spatel/dont-hardcode-public-pypi branch March 29, 2021 17:18
@asottile
Copy link
Contributor

A better solution would be to remove the argument entirely and release 2.0.0

please don't merge with unresolved reviews

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

Successfully merging this pull request may close these issues.

None yet

3 participants