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.

Sign up for GitHub

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

Jump to bottom

Remove upload to docker registry code snippets #1017

Merged
merged 9 commits into from
Dec 16, 2024
Merged

Remove upload to docker registry code snippets #1017

merged 9 commits into from
Dec 16, 2024

Conversation

soapy1
Copy link
Contributor

@soapy1 soapy1 commented Dec 10, 2024 •
edited
Loading

Follow up to #1001

Description

Following cleaning up removing docker build tasks, this PR removes the functionality for pushing created docker images to container registries and deprecating the related config. This includes:

  • removing the CONTAINER_REGISTRY build artifact type
  • removing the ContainerRegistry implementation
  • deprecating the ContainerRegistry config settings
  • updating docs + examples

Pull request checklist

  • Did you test this change locally?
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

@netlify Netlify
Copy link

netlify bot commented Dec 10, 2024 •
edited
Loading

✅ Deploy Preview for conda-store ready!

Name Link
🔨 Latest commit 785f4d4
🔍 Latest deploy log https://app.netlify.com/sites/conda-store/deploys/676084cc0140250009d013b3
😎 Deploy Preview https://deploy-preview-1017--conda-store.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@soapy1 soapy1 force-pushed the remove-upload-to-docker-registry branch 2 times, most recently from 316963a to 982a557 Compare December 10, 2024 05:44
@soapy1 soapy1 changed the title Remove upload to docker registry Remove upload to docker registry code snippets Dec 10, 2024
@soapy1 soapy1 force-pushed the remove-upload-to-docker-registry branch from cda752e to 0be6b5e Compare December 10, 2024 16:16
soapy1
soapy1 commented Dec 10, 2024
View reviewed changes
conda-store-server/conda_store_server/_internal/schema.py
CONSTRUCTOR_INSTALLER = "CONSTRUCTOR_INSTALLER"
_ = "CONTAINER_REGISTRY"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Users might have CONTAINER_REGISTRY as part of their settings (persisted to the database) for build_artifacts_kept_on_deletion. These are only updated by admins in the admin ui. They are not updated when a new conda_store config is pushed to the server (see ensure_settings function).

If the user has a build artifact type that is not in this list, then the server will error. So, we'll leave this as a valid type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea.

@soapy1 soapy1 marked this pull request as ready for review December 10, 2024 17:56
@soapy1 soapy1 requested a review from peytondmurray December 10, 2024 17:56
soapy1
soapy1 commented Dec 10, 2024
View reviewed changes
conda-store-server/conda_store_server/registry.py
config=True,
)

@default("container_registry_image_tag")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This implementation is not used since the docker build workflows have been disabled

@soapy1 soapy1 force-pushed the remove-upload-to-docker-registry branch from 329df3f to 32e9362 Compare December 11, 2024 18:01
peytondmurray
peytondmurray approved these changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@peytondmurray peytondmurray left a comment

Choose a reason for hiding this comment

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

Minor suggestions. Thanks for this 🚀

conda-store-server/conda_store_server/_internal/schema.py
CONSTRUCTOR_INSTALLER = "CONSTRUCTOR_INSTALLER"
_ = "CONTAINER_REGISTRY"
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea.

conda-store-server/conda_store_server/_internal/schema.py
history: List[DockerConfigHistory] = []
rootfs: DockerConfigRootFS


# https://docs.docker.com/registry/spec/api/#errors-2
class DockerRegistryError(enum.Enum):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it in scope to remove this as well?

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 we should keep this for now. This is used in the docker registry whenever there are errors. We are deprecating this functionality in pr #1016. But, we'll need to keep it around until the registry is fully removed.

docusaurus-docs/conda-store/references/configuration-options.md Outdated Show resolved Hide resolved
@soapy1 soapy1 force-pushed the remove-upload-to-docker-registry branch from f4bf9ad to 7b00476 Compare December 16, 2024 19:51
@soapy1 soapy1 merged commit 949c3c0 into conda-incubator:main Dec 16, 2024
30 checks passed
@soapy1 soapy1 deleted the remove-upload-to-docker-registry branch December 16, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peytondmurray peytondmurray peytondmurray approved these changes

Assignees
No one assigned
Labels
type: deprecations ⛔️
Projects
conda-store 🐍
Status: Done 💪🏾
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@soapy1 @peytondmurray