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

feat(nx-container): add support for registry option #1084

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

callajd
Copy link

@callajd callajd commented Jul 13, 2024

I went ahead and put together a small commit to enable a registry option:

All of the existing tests pass but it wasn't clear to me where I would add new tests for this option. Happy to add new tests if you could point me in the right direction.

@callajd callajd requested a review from gperdomor as a code owner July 13, 2024 20:18
@gperdomor
Copy link
Owner

Hi @callajd thank you for you interest in this project and for make this contribution, sadly I can't merge this basically because it's already covered using the tags or images options depending if you use metadata or not... The second reason why I can't merge this it's because we will break the possibility to push to multiples registries which it's also covered with tags or images options...

@callajd
Copy link
Author

callajd commented Jul 13, 2024

The problem with the existing solution is that it (whether accidentally or intentionally) shoehorns the registry concept into the distinct concepts of images and tags, which distorts the meanings of images and tags while making valid and common use cases unnecessarily difficult.

  • a registry has a collection of repositories
  • a repository is a collection of images that have tags

See for example Docker's explanation, or Google Cloud's.

Based on your comments, I've updated my PR to use 'registries' as a string[] instead of 'registry' as a single string, so this new option supports multi-registry deployment. As before, the setting remains optional so existing users can continue using the same approach for multi-registry deployment without issue.

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