-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix failing migration v67 #5849
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5849 +/- ##
==========================================
+ Coverage 37.86% 37.88% +0.01%
==========================================
Files 328 328
Lines 48318 48318
==========================================
+ Hits 18294 18303 +9
+ Misses 27393 27387 -6
+ Partials 2631 2628 -3
Continue to review full report at Codecov.
|
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.
The accessLevel function should also probably make its requests within the session too - although shouldn't deadlock.
There's a few other places in that file where X is used to perform queries within the transaction. I think we should probably just move those over too.
Needs to also be in the sess |
@zeripath I changed line 133 and line 63, which contained |
Changing line 63 broke the build, so I reverted the change on that line. |
@KubqoA I took the liberty of fixing the remaining deadlocks. Unfortunately I guess that means that I can't really LGTM this anymore... |
Thanks, so now we need to wait for someone else to LGTM this? |
Hmm. LGTM has set itself to done... So may be someone else has looked at it. @KubqoA could you just check that this migration change actually works for you though. |
@KubqoA does it work? |
How would I check if it works? I am running the docker version. Does it mean I need to create a custom Docker image and check it? |
OK I've just cherry-picked my migration test PR on top of this and it doesn't break the migration tests for v1.5.3 onwards so I'm going to say it works |
Great! So soon I can upgrade to the latest build of the Docker image and it should work? |
@zeripath can this get merged now? |
You are an awesome person @zeripath, thanks to you, my Gitea instance is again up and running without any problems so far. I am very happy for the power of open-source project communities. Keep on going! |
Fixes issue number #5848