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: Implement m2m table for container_registries, and groups #3065

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

Conversation

jopemachine
Copy link
Member

@jopemachine jopemachine commented Nov 11, 2024

Implement AssociationContainerRegistriesGroups as m2m table of container_registries, and groups for resolving #1908.

Note

#1908 mentions both user and project, but since project needs to be implemented first, this PR stack addresses only the project aspect.
Refer to #1960 for adding an association table with the User table and related tasks.

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Mention to the original issue

@github-actions github-actions bot added comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated labels Nov 11, 2024
depends_on = None


def upgrade() -> None:
Copy link
Member Author

@jopemachine jopemachine Nov 11, 2024

Choose a reason for hiding this comment

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

Previously, there was no per-project image lookup functionality, so data migration logic will be unnecessary here.

@jopemachine jopemachine marked this pull request as ready for review November 11, 2024 05:57
@jopemachine jopemachine changed the title feat: Implement AssociationContainerRegistriesGroupsRow feat: Implement m2m table for container_registries, and groups Nov 11, 2024
@jopemachine jopemachine force-pushed the topic/11-11-feat_implement_associationcontainerregistriesgroupsrow_ branch from b255920 to c1c349c Compare November 13, 2024 06:29
@jopemachine jopemachine force-pushed the topic/11-11-feat_implement_associationcontainerregistriesgroupsrow_ branch from b2abda9 to 6004978 Compare November 14, 2024 06:00
Copy link
Member

@fregataa fregataa left a comment

Choose a reason for hiding this comment

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

How about adding the new table definition in an existing file such as models/container_registries.py or models/groups.py? This is just a suggestion, you don't need to reflect it right away.

@jopemachine jopemachine force-pushed the topic/11-11-feat_implement_associationcontainerregistriesgroupsrow_ branch from d4ce974 to bebc68e Compare November 18, 2024 01:28
@jopemachine
Copy link
Member Author

How about adding the new table definition in an existing file such as models/container_registries.py or models/groups.py? This is just a suggestion, you don't need to reflect it right away.

Personally, I think it’s better to place them in separate files.
Having a single file for each table, rather than defining multiple tables in one file, seems more intuitive and makes it easier to refer back to them later.

@jopemachine jopemachine force-pushed the topic/11-11-feat_implement_associationcontainerregistriesgroupsrow_ branch from 5e6f6ed to 5cad46c Compare November 20, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:M 30~100 LoC type:feature Add new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Per-project container registry mapping (or RBAC)
2 participants