Skip to content

Conversation

daviian
Copy link
Member

@daviian daviian commented Sep 25, 2017

Backport of #2602

daviian and others added 4 commits September 25, 2017 19:05
* change repoUnit model in migration * fix v16 migration repo_unit table * fix lint error * move type definition inside function Signed-off-by: David Schneiderbauer <dschneiderbauer@gmail.com>
Signed-off-by: David Schneiderbauer <dschneiderbauer@gmail.com>
@codecov-io
Copy link

codecov-io commented Sep 25, 2017

Codecov Report

Merging #2604 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@ ## master #2604 +/- ## ======================================= Coverage 27.13% 27.13% ======================================= Files 86 86 Lines 17062 17062 ======================================= Hits 4629 4629 Misses 11755 11755 Partials 678 678

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 92123fe...d53ab74. Read the comment docs.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 25, 2017
@lunny lunny added this to the 1.3.0 milestone Sep 26, 2017
@lunny lunny added the type/bug label Sep 26, 2017
@daviian
Copy link
Member Author

daviian commented Sep 27, 2017

Currently there is an issue in v39 (introduced in this PR) which needs to be fixed before merging

@lafriks
Copy link
Member

lafriks commented Sep 27, 2017

@daviian what issue?

@daviian
Copy link
Member Author

daviian commented Sep 27, 2017

@lafriks https://github.com/daviian/gitea/blob/c9430b29a5220e2297aa9b93b70a25062f831509/models/migrations/v39.go#L73
Xorm doesn't return anything here, despite there should be 96 units in my testdata with UnitTypeIssue

lafriks and others added 7 commits September 28, 2017 09:30
* change repoUnit model in migration * fix v16 migration repo_unit table * fix lint error * move type definition inside function Signed-off-by: David Schneiderbauer <dschneiderbauer@gmail.com>
Signed-off-by: David Schneiderbauer <dschneiderbauer@gmail.com>
Signed-off-by: David Schneiderbauer <dschneiderbauer@gmail.com>
@daviian daviian force-pushed the backport/migration-fix branch from c840545 to 54f684d Compare September 28, 2017 19:22
@lafriks lafriks changed the title Backport of #2602 Rewrite migrations to not depend on future code changes Sep 30, 2017
users := make([]*models.User, 0, batchSize)
if err := x.Limit(start, batchSize).Find(users); err != nil {
users := make([]*User, 0, batchSize)
if err := x.Limit(batchSize, start).Find(&users); err != nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Nearly impossible to see

@strk
Copy link
Member

strk commented Oct 4, 2017

Any chance to see an automated test for migration from a dump of a Gogs0.99 database ?

@lafriks
Copy link
Member

lafriks commented Oct 4, 2017

@strk not in the scope of this PR but otherwise I agree we would need that but I currently have no idea how to achieve that

@strk
Copy link
Member

strk commented Oct 4, 2017 via email

@tboerger tboerger removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 4, 2017
@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 4, 2017
@lafriks
Copy link
Member

lafriks commented Oct 4, 2017

@strk me and @daviian have tested it multiple times

@lafriks lafriks modified the milestones: 1.3.0, 1.2.0 Oct 5, 2017
@mrexodia
Copy link
Contributor

mrexodia commented Oct 6, 2017

Just tried this on a database with version=17 and it appears to have worked! (raspberry pi zero w, had to change swap size to 1024mb to compile)

# Make errors about go-bindata go away go get github.com/jteeuwen/go-bindata cd $GOPATH/src/github.com/jteeuwen/go-bindata/go-bindata go build export PATH=$PATH:$GOPATH/src/github.com/jteeuwen/go-bindata/go-bindata # Get gitea go get -d -u code.gitea.io/gitea cd $GOPATH/src/code.gitea.io/gitea # Get PR 2604 that fixes migration issues git fetch origin pull/2604/head:pr-2604 git checkout pr-2604 # Build gitea (will take some time) TAGS="bindata sqlite" make generate build 

I kind of followed https://docs.gitea.io/en-us/install-from-source/ and https://docs.gitea.io/en-us/upgrade-from-gogs/ and @lafriks told me to change the gogs.db with the following SQL command: update version set version=14; which may or may not have contributed to this being successful...

@daviian
Copy link
Member Author

daviian commented Oct 6, 2017

@mrexodia Glad to here that 😃 Setting the version to 14 is necessary as otherwise required migrations (in your example 14 to 17) are not applied to the database.

@Morlinest
Copy link
Member

LGTM, code looks good, trusting manual tests of @lafriks & @daviian

@tboerger tboerger 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 Oct 7, 2017
@lafriks
Copy link
Member

lafriks commented Oct 7, 2017

Make LG-TM work again

@daviian daviian force-pushed the backport/migration-fix branch from a5e38d2 to d53ab74 Compare October 8, 2017 09:13
@lunny lunny modified the milestones: 1.2.0, 1.3.0 Oct 8, 2017
@lunny lunny merged commit ebac051 into go-gitea:master Oct 8, 2017
@daviian
Copy link
Member Author

daviian commented Oct 8, 2017

@lunny @lafriks Should I create a backport to 1.2?

@daviian daviian deleted the backport/migration-fix branch October 8, 2017 12:04
@lafriks
Copy link
Member

lafriks commented Oct 8, 2017

@daviian sure, go ahead

@lafriks lafriks added the backport/done All backports for this PR have been created label Oct 8, 2017
daviian added a commit to daviian/gitea that referenced this pull request Oct 9, 2017
* v38 migration used an outdated version of RepoUnit model (go-gitea#2602) * change repoUnit model in migration * fix v16 migration repo_unit table * fix lint error * move type definition inside function * Fix migration from Gogs * Refactor code * add error check * Additiomal fixes for migrations * Add back nil check
lafriks pushed a commit that referenced this pull request Oct 9, 2017
* Rewrite migrations to not depend on future code changes (#2604) * v38 migration used an outdated version of RepoUnit model (#2602) * change repoUnit model in migration * fix v16 migration repo_unit table * fix lint error * move type definition inside function * Fix migration from Gogs * Refactor code * add error check * Additiomal fixes for migrations * Add back nil check * replace deprecated .Id with .ID Signed-off-by: David Schneiderbauer <dschneiderbauer@gmail.com> * change string map to interface map Signed-off-by: David Schneiderbauer <dschneiderbauer@gmail.com>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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

8 participants