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

refactor: Remove the foreign key constraint of vfolders.{user,group} #2404

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

achimnol
Copy link
Member

@achimnol achimnol commented Jul 8, 2024

refs #2376

Remove the foreign/check constraints preventing decoupling of user/vfolder deletions.

Checklist: (if applicable)

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

Copy link

graphite-app bot commented Jul 8, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Member Author

achimnol commented Jul 8, 2024

@github-actions github-actions bot added comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:S 10~30 LoC labels Jul 8, 2024
@github-actions github-actions bot added size:M 30~100 LoC and removed size:S 10~30 LoC labels Jul 8, 2024
@achimnol achimnol added type:refactor Refactor codes or add tests. urgency:4 As soon as feasible, implementation is essential. labels Jul 8, 2024
@achimnol achimnol force-pushed the topic/remove-users-vfolders-freignkey branch from ed27ce0 to 0c35a76 Compare July 8, 2024 15:42
@achimnol achimnol added this to the 24.03 milestone Jul 8, 2024
@achimnol achimnol marked this pull request as ready for review July 8, 2024 15:52
@achimnol achimnol requested a review from fregataa July 8, 2024 15:58
Comment on lines +28 to +38
op.create_check_constraint(
"ck_vfolders_ownership_type_match_with_user_or_group",
"vfolders",
"(ownership_type = 'user' AND \"user\" IS NOT NULL) OR "
"(ownership_type = 'group' AND \"group\" IS NOT NULL)",
)
op.create_check_constraint(
"ck_vfolders_either_one_of_user_or_group",
"vfolders",
'("user" IS NULL AND "group" IS NOT NULL) OR ("user" IS NOT NULL AND "group" IS NULL)',
)
Copy link
Member

Choose a reason for hiding this comment

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

How should we handle integrity errors which can occur if some vfolder records have null values for both user and group columns?

Copy link
Member

@fregataa fregataa Jul 9, 2024

Choose a reason for hiding this comment

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

We discussed about this issue and we will cover it by introducing placeholder user (a.k.a ghost user)

@fregataa fregataa self-requested a review July 9, 2024 05:31
Copy link

graphite-app bot commented Jul 9, 2024

Merge activity

  • Jul 9, 4:30 AM EDT: The merge label 'flow:merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 9, 4:30 AM EDT: achimnol added this pull request to the Graphite merge queue.
  • Jul 9, 4:34 AM EDT: achimnol merged this pull request with the Graphite merge queue.

…2404)

refs #2376

Remove the foreign/check constraints preventing decoupling of user/vfolder deletions.

**Checklist:** (if applicable)

- [x] Milestone metadata specifying the target backport version
- [x] Mention to the original issue
@achimnol achimnol force-pushed the topic/remove-users-vfolders-freignkey branch from 53f34cb to 82b47f2 Compare July 9, 2024 08:31
@graphite-app graphite-app bot merged commit 82b47f2 into main Jul 9, 2024
29 checks passed
@graphite-app graphite-app bot deleted the topic/remove-users-vfolders-freignkey branch July 9, 2024 08:34
lablup-octodog pushed a commit that referenced this pull request Jul 9, 2024
…2404)

refs #2376

Remove the foreign/check constraints preventing decoupling of user/vfolder deletions.

**Checklist:** (if applicable)

- [x] Milestone metadata specifying the target backport version
- [x] Mention to the original issue

Backported-from: main (24.09)
Backported-to: 24.03
Backport-of: 2404
lablup-octodog added a commit that referenced this pull request Jul 13, 2024
…2404) (#2406)

refs #2376

Co-authored-by: achimnol <[email protected]>
Remove the foreign/check constraints preventing decoupling of user/vfolder deletions.
Backported-from: main (24.09)
Backported-to: 24.03
Backport-of: 2404
jopemachine pushed a commit that referenced this pull request Jul 14, 2024
…2404)

refs #2376

Remove the foreign/check constraints preventing decoupling of user/vfolder deletions.

**Checklist:** (if applicable)

- [x] Milestone metadata specifying the target backport version
- [x] Mention to the original issue
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 flow:merge-queue require:db-migration Automatically set when alembic migrations are added or updated size:M 30~100 LoC type:refactor Refactor codes or add tests. urgency:4 As soon as feasible, implementation is essential.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants