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.

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

Run Migrate in Install rather than just SyncTables #17475

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Oct 28, 2021

The underlying problem in #17328 appears to be that users are re-running the install
page during upgrades. The function that tests and creates the db did not intend for
this and thus instead the migration scripts being run - a simple sync tables occurs.

This then causes a weird partially migrated DB which causes, in this release cycle,
the duplicate column in task table error. It is likely the cause of some weird
partial migration errors in other cycles too.

This PR simply ensures that the migration scripts are also run at this point too.

Fix #17328
Fix #16828

Signed-off-by: Andrew Thornton art27@cantab.net

The underlying problem in go-gitea#17328 appears to be that users are re-running the install
page during upgrades. The function that tests and creates the db did not intend for
this and thus instead the migration scripts being run - a simple sync tables occurs.

This then causes a weird partially migrated DB which causes, in this release cycle,
the duplicate column in task table error. It is likely the cause of some weird
partial migration errors in other cycles too.

This PR simply ensures that the migration scripts are also run at this point too.

Fix go-gitea#17328

Signed-off-by: Andrew Thornton <art27@cantab.net>
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 28, 2021
Signed-off-by: Andrew Thornton <art27@cantab.net>
@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 Oct 29, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #17475 (662dcfa) into main (0b4a8be) will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17475      +/-   ##
==========================================
+ Coverage   45.47%   45.49%   +0.01%     
==========================================
  Files         791      791              
  Lines       88697    88702       +5     
==========================================
+ Hits        40335    40352      +17     
+ Misses      41850    41836      -14     
- Partials     6512     6514       +2     
Impacted Files Coverage Δ
models/db/engine.go 30.07% <0.00%> (-1.18%) ⬇️
routers/install/install.go 0.66% <0.00%> (ø)
routers/web/feed/convert.go 0.00% <0.00%> (ø)
modules/queue/queue_channel.go 95.00% <0.00%> (-1.67%) ⬇️
models/repo_list.go 78.43% <0.00%> (-0.79%) ⬇️
services/pull/pull.go 42.19% <0.00%> (+0.40%) ⬆️
routers/web/repo/view.go 40.20% <0.00%> (+0.51%) ⬆️
modules/queue/workerpool.go 50.76% <0.00%> (+2.67%) ⬆️
models/unit.go 43.83% <0.00%> (+2.73%) ⬆️
modules/queue/queue_bytefifo.go 63.47% <0.00%> (+3.59%) ⬆️

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 9733b60...662dcfa. Read the comment docs.

@lunny
Copy link
Member

lunny commented Oct 29, 2021

It seems NewTestEngine could be replaced by NewEngine.

@zeripath
Copy link
Contributor Author

It seems NewTestEngine could be replaced by NewEngine.

I think there's a subtle logging difference.

@zeripath zeripath merged commit e6e3b21 into go-gitea:main Oct 29, 2021
@zeripath zeripath deleted the fix-17328-always-run-migrate-not-syncTables-even-in-install branch October 29, 2021 08:23
zeripath added a commit to zeripath/gitea that referenced this pull request Oct 29, 2021
Backport go-gitea#17475

The underlying problem in go-gitea#17328 appears to be that users are re-running the install
page during upgrades. The function that tests and creates the db did not intend for
this and thus instead the migration scripts being run - a simple sync tables occurs.

This then causes a weird partially migrated DB which causes, in this release cycle,
the duplicate column in task table error. It is likely the cause of some weird
partial migration errors in other cycles too.

This PR simply ensures that the migration scripts are also run at this point too.

Fix go-gitea#17328

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the backport/done All backports for this PR have been created label Oct 29, 2021
zeripath added a commit that referenced this pull request Oct 30, 2021
Backport #17475

The underlying problem in #17328 appears to be that users are re-running the install
page during upgrades. The function that tests and creates the db did not intend for
this and thus instead the migration scripts being run - a simple sync tables occurs.

This then causes a weird partially migrated DB which causes, in this release cycle,
the duplicate column in task table error. It is likely the cause of some weird
partial migration errors in other cycles too.

This PR simply ensures that the migration scripts are also run at this point too.

Fix #17328

Signed-off-by: Andrew Thornton <art27@cantab.net>
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
* Run Migrate in Install rather than just SyncTables

The underlying problem in go-gitea#17328 appears to be that users are re-running the install
page during upgrades. The function that tests and creates the db did not intend for
this and thus instead the migration scripts being run - a simple sync tables occurs.

This then causes a weird partially migrated DB which causes, in this release cycle,
the duplicate column in task table error. It is likely the cause of some weird
partial migration errors in other cycles too.

This PR simply ensures that the migration scripts are also run at this point too.

Fix go-gitea#17328

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
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
Projects
None yet
5 participants