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

Add support for composite unique constraint in auto migration #984

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

atkei
Copy link

@atkei atkei commented Apr 12, 2024

Related to #172, #175 and based on #957.

Add Constraint and UniqueConstraint to support composite unique constraint in auto migraiton.

Some work remains such as adding doc and testing, but I would like to have feedback on my proposal first.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 95.85253% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 89.72%. Comparing base (363d683) to head (07f8b94).
Report is 1 commits behind head on master.

Files Patch % Lines
piccolo/apps/migrations/auto/schema_differ.py 92.85% 3 Missing ⚠️
piccolo/apps/migrations/auto/migration_manager.py 97.36% 2 Missing ⚠️
piccolo/apps/migrations/auto/diffable_table.py 95.00% 1 Missing ⚠️
piccolo/apps/migrations/auto/schema_snapshot.py 80.00% 1 Missing ⚠️
piccolo/constraint.py 96.96% 1 Missing ⚠️
piccolo/table.py 90.90% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #984      +/-   ##
==========================================
- Coverage   92.78%   89.72%   -3.07%     
==========================================
  Files         108      109       +1     
  Lines        8182     8399     +217     
==========================================
- Hits         7592     7536      -56     
- Misses        590      863     +273     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sinisaos
Copy link
Member

@dantownsend I hope it's not a problem, but I have approved and run the workflow for this PR. I have already tried these changes in the @atkei local branch and in my case everything works great as I wrote in this comment. Can you try and review this PR. Thanks.

@dantownsend
Copy link
Member

@sinisaos Awesome thanks - feel free to run the CI whenever you want.

@atkei Thanks a lot for this - looks great!

@AlexanderMakarov
Copy link

@sinisaos @dantownsend - could you make a hint - when it would be merged? Would it mean new version/release?

@dantownsend
Copy link
Member

@AlexanderMakarov I'll try my best to review it properly this week.

@MoSheikh
Copy link

Currenty looking for this funtionality - would appreciate it someone on the team would be able to review!

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.

None yet

6 participants