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

Don't hard-code legacy yelpcorp PyPI registry, remove pip-custom-platform #3664

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

chriskuehl
Copy link
Contributor

@chriskuehl chriskuehl commented Jul 31, 2023

Internal ticket: CORESERV-12777

The general idea is to (1) remove pip-custom-platform, (2) stop hard-coding the PyPI registry URL in places where it's possible, and (3) hard-code the platformed registry at least if we have to.

We set the registry URL globally in /etc/pip.conf on Yelp hosts and inside Yelp's Docker images, so we should be able to get away with not specifying it in most locations. This also allows us to take advantage of the platform-specific registries which no longer need pip-custom-platform. Unfortunately it is not so easy to rely on this for many of the Docker images so I had to hard-code the platform-specific registry.

Same idea as Yelp/Tron#921.

Passed locally on a yelpcorp host:

  • make test
  • make itest
  • make k8s_itests
  • make itest_jammy

@chriskuehl chriskuehl changed the title Don't hard-code yelpcorp PyPI registry Don't hard-code yelpcorp PyPI registry, remove pip-custom-platform Jul 31, 2023
@chriskuehl chriskuehl changed the title Don't hard-code yelpcorp PyPI registry, remove pip-custom-platform Don't hard-code legacy yelpcorp PyPI registry, remove pip-custom-platform Jul 31, 2023
Internal ticket: CORESERV-12777

Same idea as Yelp/Tron#921
@chriskuehl chriskuehl marked this pull request as ready for review July 31, 2023 19:11
@@ -15,7 +15,7 @@
ARG DOCKER_REGISTRY=docker-dev.yelpcorp.com/
FROM ${DOCKER_REGISTRY}xenial_pkgbuild

ARG PIP_INDEX_URL=https://pypi.yelpcorp.com/simple
ARG PIP_INDEX_URL=https://pypi.yelpcorp.com/xenial/simple
Copy link
Member

Choose a reason for hiding this comment

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

are these still needed (here and elsewhere)?

Copy link
Member

Choose a reason for hiding this comment

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

Re-read the description and I see, this is needed and intentionally here (though I'm not 100% clear why)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it may not be needed on this one since it uses our internal pkgbuild image, but for most of these it is still needed due to using the public Ubuntu base images. (Even though they all pull from our registry, they're pulling mirrored Ubuntu images as opposed to our internal modified Ubuntu images.)

In the near future (hopefully this quarter) we should be able to consolidate on the main root registry again (and without requiring pip-custom-platform) so we can do a followup round later to clean all of these up even more.

@chriskuehl chriskuehl merged commit a76edf5 into master Jul 31, 2023
6 checks passed
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