Rewrite migrations to not depend on future code changes #2604

daviian · 2017-09-25T17:07:02Z

Backport of #2602

* change repoUnit model in migration * fix v16 migration repo_unit table * fix lint error * move type definition inside function Signed-off-by: David Schneiderbauer <[email protected]>

Signed-off-by: David Schneiderbauer <[email protected]>

codecov-io · 2017-09-25T22:16:10Z

Codecov Report

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

@@           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.

daviian · 2017-09-27T08:55:57Z

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

lafriks · 2017-09-27T12:06:49Z

@daviian what issue?

daviian · 2017-09-27T12:16:41Z

@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

* change repoUnit model in migration * fix v16 migration repo_unit table * fix lint error * move type definition inside function Signed-off-by: David Schneiderbauer <[email protected]>

Signed-off-by: David Schneiderbauer <[email protected]>

…o daviian-backport/migration-fix

daviian · 2017-09-30T18:58:07Z

models/migrations/v37.go

-		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 {


Good catch. Nearly impossible to see

strk · 2017-10-04T07:47:05Z

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

lafriks · 2017-10-04T09:21:26Z

@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 · 2017-10-04T10:24:04Z

Trusted/untested LGTM - please make sure to manually test it !

lafriks · 2017-10-04T10:25:21Z

@strk me and @daviian have tested it multiple times

mrexodia · 2017-10-06T20:55:05Z

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 · 2017-10-06T21:17:09Z

@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 · 2017-10-07T15:13:52Z

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

lafriks · 2017-10-07T15:25:36Z

Make LG-TM work again

daviian · 2017-10-08T12:04:04Z

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

lafriks · 2017-10-08T12:19:31Z

@daviian sure, go ahead

* 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

* 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 <[email protected]> * change string map to interface map Signed-off-by: David Schneiderbauer <[email protected]>

daviian and others added 4 commits September 25, 2017 19:05

v38 migration used an outdated version of RepoUnit model (go-gitea#2602)

bc80566

* change repoUnit model in migration * fix v16 migration repo_unit table * fix lint error * move type definition inside function Signed-off-by: David Schneiderbauer <[email protected]>

fix lint error

741af60

Signed-off-by: David Schneiderbauer <[email protected]>

Fix time tracking migration

179333c

Refactor code

c9430b2

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 25, 2017

lunny added this to the 1.3.0 milestone Sep 26, 2017

lunny added the type/bug label Sep 26, 2017

lafriks and others added 7 commits September 28, 2017 09:30

Fix migration from Gogs

c840545

v38 migration used an outdated version of RepoUnit model (go-gitea#2602)

bfb97d5

* change repoUnit model in migration * fix v16 migration repo_unit table * fix lint error * move type definition inside function Signed-off-by: David Schneiderbauer <[email protected]>

fix lint error

9e22110

Signed-off-by: David Schneiderbauer <[email protected]>

Fix time tracking migration

6035eda

Refactor code

fb73cb0

Fix migration from Gogs

dd48773

add error check

54f684d

Signed-off-by: David Schneiderbauer <[email protected]>

daviian force-pushed the backport/migration-fix branch from c840545 to 54f684d Compare September 28, 2017 19:22

lafriks added 2 commits September 30, 2017 15:41

Merge branch 'backport/migration-fix' of github.com:daviian/gitea int…

828d168

…o daviian-backport/migration-fix

Additiomal fixes for migrations

2260204

lafriks added the backport/v1.2 label Sep 30, 2017

lafriks added 2 commits September 30, 2017 21:32

Fix timetracking migration

85e8edd

Add back nil check

83e65f2

lafriks changed the title ~~Backport of #2602~~ Rewrite migrations to not depend on future code changes Sep 30, 2017

daviian commented Sep 30, 2017

View reviewed changes

tboerger removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 4, 2017

tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 4, 2017

Merge branch 'master' into backport/migration-fix

efa482d

lafriks modified the milestones: 1.3.0, 1.2.0 Oct 5, 2017

mrexodia mentioned this pull request Oct 6, 2017

cannot allocate memory after opening truncated diff (possible memory leak) #2669

Closed

7 tasks

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

Merge branch 'master' into backport/migration-fix

d53ab74

daviian force-pushed the backport/migration-fix branch from a5e38d2 to d53ab74 Compare October 8, 2017 09:13

lunny modified the milestones: 1.2.0, 1.3.0 Oct 8, 2017

lunny merged commit ebac051 into go-gitea:master Oct 8, 2017

daviian deleted the backport/migration-fix branch October 8, 2017 12:04

daviian mentioned this pull request Oct 8, 2017

Backport of migration fixes (#2604) #2677

Merged

lafriks added the backport/done All backports for this PR have been created label Oct 8, 2017

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite migrations to not depend on future code changes #2604

Rewrite migrations to not depend on future code changes #2604

daviian commented Sep 25, 2017

codecov-io commented Sep 25, 2017 •

edited

Loading

daviian commented Sep 27, 2017 •

edited

Loading

lafriks commented Sep 27, 2017

daviian commented Sep 27, 2017

daviian Sep 30, 2017

strk commented Oct 4, 2017

lafriks commented Oct 4, 2017

strk commented Oct 4, 2017 via email

lafriks commented Oct 4, 2017

mrexodia commented Oct 6, 2017 •

edited

Loading

daviian commented Oct 6, 2017

Morlinest commented Oct 7, 2017

lafriks commented Oct 7, 2017

daviian commented Oct 8, 2017

lafriks commented Oct 8, 2017

Rewrite migrations to not depend on future code changes #2604

Rewrite migrations to not depend on future code changes #2604

Conversation

daviian commented Sep 25, 2017

codecov-io commented Sep 25, 2017 • edited Loading

Codecov Report

daviian commented Sep 27, 2017 • edited Loading

lafriks commented Sep 27, 2017

daviian commented Sep 27, 2017

daviian Sep 30, 2017

Choose a reason for hiding this comment

strk commented Oct 4, 2017

lafriks commented Oct 4, 2017

strk commented Oct 4, 2017 via email

lafriks commented Oct 4, 2017

mrexodia commented Oct 6, 2017 • edited Loading

daviian commented Oct 6, 2017

Morlinest commented Oct 7, 2017

lafriks commented Oct 7, 2017

daviian commented Oct 8, 2017

lafriks commented Oct 8, 2017

codecov-io commented Sep 25, 2017 •

edited

Loading

daviian commented Sep 27, 2017 •

edited

Loading

mrexodia commented Oct 6, 2017 •

edited

Loading