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.

Sign up for GitHub

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

Jump to bottom

Fix migration v141 #14387

Merged
merged 24 commits into from
Jan 28, 2021
Merged

Fix migration v141 #14387

merged 24 commits into from
Jan 28, 2021

Conversation

6543
Copy link
Member

@6543 6543 commented Jan 18, 2021 •
edited
Loading

make filed same as in models ...

fix #14069

@6543 6543 added this to the 1.14.0 milestone Jan 18, 2021
@6543 6543 added the type/bug label Jan 18, 2021
@6543 6543 changed the title Fix mig 141 Fix migration v141 Jan 18, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Jan 18, 2021
@6543 6543 mentioned this pull request Jan 18, 2021
Closed
7 tasks
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 18, 2021
@6543 6543 added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Jan 18, 2021
@6543 6543 mentioned this pull request Jan 18, 2021
Merged
@6543
Copy link
Member Author

6543 commented Jan 18, 2021

Backport: #14388

@6543 6543 added the backport/done All backports for this PR have been created label Jan 18, 2021
@6543 6543 requested review from lafriks and zeripath January 18, 2021 18:06
@6543
Copy link
Member Author

6543 commented Jan 18, 2021

added migration ... @lafriks @zeripath need 👀

@6543 6543 removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Jan 18, 2021
@codecov-io
Copy link

codecov-io commented Jan 18, 2021 •
edited
Loading

Codecov Report

Merging #14387 (245b671) into master (525d75c) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14387      +/-   ##
==========================================
- Coverage   41.76%   41.74%   -0.03%     
==========================================
  Files         748      749       +1     
  Lines       80103    80171      +68     
==========================================
+ Hits        33455    33465      +10     
- Misses      41125    41187      +62     
+ Partials     5523     5519       -4
Impacted Files Coverage Δ
models/migrations/migrations.go 2.28% <ø> (ø)
models/migrations/v141.go 0.00% <0.00%> (ø)
models/migrations/v167.go 0.00% <0.00%> (ø)
models/gpg_key.go 53.33% <0.00%> (-0.58%) ⬇️
modules/git/batch_reader_nogogit.go 46.26% <0.00%> (+1.49%) ⬆️
modules/process/manager.go 75.00% <0.00%> (+2.50%) ⬆️
modules/git/repo_language_stats_nogogit.go 63.82% <0.00%> (+6.38%) ⬆️
modules/indexer/stats/db.go 68.00% <0.00%> (+12.00%) ⬆️
modules/util/timer.go 85.71% <0.00%> (+42.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 800c436...245b671. Read the comment docs.

@zeripath
Copy link
Contributor

I think we should add a doctor command to deal with the nulls

@lafriks
Copy link
Member

lafriks commented Jan 19, 2021

Isn't it possible to use alter to change not null and default?

@lunny
Copy link
Member

lunny commented Jan 19, 2021 •
edited
Loading

A recreate is dangerous that I don't think we should do that frequently. Or we could make this as a doctor command and mention user should back up some tables.

lunny
lunny reviewed Jan 19, 2021
View reviewed changes
models/migrations/v167.go Outdated Show resolved Hide resolved
@6543 6543 added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Jan 20, 2021
zeripath
zeripath reviewed Jan 20, 2021
View reviewed changes
models/migrations/v167.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 20, 2021
6543
6543 commented Jan 20, 2021
View reviewed changes
models/migrations/v167.go Outdated Show resolved Hide resolved
@6543
Copy link
Member Author

6543 commented Jan 22, 2021

@techknowlogick now it use the use 2x add col & 2x update & 2x drop col algo

@6543
Copy link
Member Author

6543 commented Jan 23, 2021 •
edited
Loading

@zeripath now this cause somehow a DB deadlock :/
https://drone.gitea.io/go-gitea/gitea/35207/3/5
https://drone.gitea.io/go-gitea/gitea/35207/2/14

So It's not near to be done :(

lafriks pushed a commit that referenced this pull request Jan 23, 2021
@6543
Fix migration v141 (#14387) (#14388)
00dc35e 
* Fix mig 141

* temporary fix dump
@6543
Copy link
Member Author

6543 commented Jan 28, 2021

.

@6543 6543 merged commit 2ebe609 into go-gitea:master Jan 28, 2021
@6543 6543 deleted the fix-db-migration_141 branch January 28, 2021 22:58
@zeripath zeripath mentioned this pull request Feb 7, 2021
Closed
6 tasks
@go-gitea go-gitea locked and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny left review comments

@zeripath zeripath zeripath approved these changes

@techknowlogick techknowlogick techknowlogick left review comments

@lafriks lafriks Awaiting requested review from lafriks

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Milestone
1.14.0
Development

Successfully merging this pull request may close these issues.

Users become "Ghost" when restoring from dump
7 participants
@6543 @codecov-io @zeripath @lafriks @lunny @techknowlogick @GiteaBot