-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
ref(hc): Advisory lock #57958
ref(hc): Advisory lock #57958
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. We're handling the lock acquisition errors now and handling errors during unlocking as well.
@@ -76,7 +76,7 @@ def get_conflicting_unique_columns( | |||
scope = category.get_scope() | |||
scope_controlled_columns: List[str] | |||
if scope == scope.USER_SCOPE: | |||
scope_controlled_columns = [get_foreign_key_column(destination, User)] | |||
scope_controlled_columns = ["ident", get_foreign_key_column(destination, User)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth leaving a comment here about this being for AuthIdentity
? I would see this causing problems in the future if we have another user scope replicated model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct! I'll add a comment on my next pass.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
PR reverted: 4b442eb |
This reverts commit fa0cf78. Co-authored-by: corps <593850+corps@users.noreply.github.com>
Pass 2 on #57877
Added better handling for operational error on lock timeout (something lost in the original PR), and also fixed the outbox contention issues around auth identity.