Make Migrations Cancelable #12917

6543 · 2020-09-21T14:50:22Z

routers/repo/migrate.go

codecov-commenter · 2020-09-21T16:08:45Z

Codecov Report

Merging #12917 into master will increase coverage by 0.11%.
The diff coverage is 7.14%.

@@            Coverage Diff             @@
##           master   #12917      +/-   ##
==========================================
+ Coverage   42.49%   42.60%   +0.11%     
==========================================
  Files         672      672              
  Lines       73785    73799      +14     
==========================================
+ Hits        31353    31443      +90     
+ Misses      37361    37263      -98     
- Partials     5071     5093      +22

Impacted Files	Coverage Δ
routers/repo/migrate.go	`37.58% <0.00%> (-3.82%)`	⬇️
routers/routes/routes.go	`89.08% <100.00%> (+0.01%)`	⬆️
modules/queue/workerpool.go	`58.77% <0.00%> (-2.05%)`	⬇️
services/pull/pull.go	`41.27% <0.00%> (+0.49%)`	⬆️
modules/log/event.go	`59.43% <0.00%> (+0.94%)`	⬆️
services/mailer/mail.go	`55.91% <0.00%> (+1.07%)`	⬆️
modules/notification/notification.go	`81.25% <0.00%> (+2.67%)`	⬆️
modules/notification/base/null.go	`71.42% <0.00%> (+2.85%)`	⬆️
modules/git/commit.go	`54.72% <0.00%> (+4.05%)`	⬆️
modules/notification/mail/mail.go	`34.48% <0.00%> (+5.74%)`	⬆️
... and 2 more

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 a23c128...6b4e015. Read the comment docs.

codecov-io · 2020-10-07T11:00:43Z

Codecov Report

Merging #12917 (cfed3be) into master (d8c2c6c) will increase coverage by 0.03%.
The diff coverage is 10.20%.

@@            Coverage Diff             @@
##           master   #12917      +/-   ##
==========================================
+ Coverage   42.22%   42.26%   +0.03%     
==========================================
  Files         708      709       +1     
  Lines       77173    77221      +48     
==========================================
+ Hits        32589    32634      +45     
+ Misses      39225    39223       -2     
- Partials     5359     5364       +5

Impacted Files	Coverage Δ
models/migrations/migrations.go	`2.44% <ø> (ø)`
models/migrations/v161.go	`0.00% <0.00%> (ø)`
models/task.go	`33.03% <0.00%> (-1.87%)`	⬇️
routers/repo/migrate.go	`33.74% <0.00%> (-8.57%)`	⬇️
modules/task/migrate.go	`32.39% <50.00%> (+0.96%)`	⬆️
modules/process/manager.go	`75.90% <100.00%> (+3.40%)`	⬆️
routers/routes/macaron.go	`92.54% <100.00%> (+<0.01%)`	⬆️
modules/queue/workerpool.go	`58.77% <0.00%> (-2.05%)`	⬇️
modules/queue/unique_queue_disk_channel.go	`53.84% <0.00%> (-1.54%)`	⬇️
modules/git/repo.go	`45.72% <0.00%> (-1.01%)`	⬇️
... and 11 more

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 d8c2c6c...cfed3be. Read the comment docs.

templates/repo/migrate/migrating.tmpl

routers/repo/migrate.go

routers/routes/routes.go

templates/repo/migrate/migrating.tmpl

routers/routes/routes.go

routers/repo/migrate.go

6543 · 2020-11-28T22:56:57Z

marked as ready, as per https://github.com/go-gitea/gitea/pull/12917/files#r527381336 & 00e532e

silverwind · 2020-11-28T23:03:53Z

What is this | in your screenshot?

6543 · 2020-11-28T23:25:22Z

What is this | in your screenshot?

the cursur 😆

silverwind · 2020-11-28T23:38:08Z

Shouldn't a button show cursor: pointer? Where is cursor: text coming from? (Yeah, I'm too lazy to test myself right now)

6543 · 2020-11-28T23:59:54Z

@silverwind if you like to have a "fresh" screenshot (00e532e460e790d5449931ceec0f718435de9261):

routers/repo/migrate.go

templates/repo/migrate/migrating.tmpl

delvh · 2021-09-28T12:31:31Z

models/migrations/v161.go

@@ -0,0 +1,17 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.


Well, this file certainly needs to be renamed until this PR will be merged.

delvh · 2021-09-28T12:35:31Z

models/task.go

+	if _, err := fmt.Sscanf(task.Process, "%d/%d", &guid, &pid); err != nil {
+		return 0, 0
+	}
+	return


Does that compile?
To me, this seems like a syntax error.
This whole function seems not finished.
Could it be that you intended to finish it another time?

If it compiles, I can understand why, but I wouldn't have dreamed up such a syntax ever…

it do compile :)

https://play.golang.org/p/Ei7xfQpJFTn

delvh · 2021-09-28T12:45:39Z

models/task.go

@@ -114,6 +115,17 @@ func (task *Task) MigrateConfig() (*migration.MigrateOptions, error) {
 	return nil, fmt.Errorf("Task type is %s, not Migrate Repo", task.Type.Name())
 }

+// GUIDandPID return GUID and PID of a running task
+func (task *Task) GUIDandPID() (guid int64, pid int64) {


I think simply returning 0, 0 is not always the best idea.
What if you extended the return to produce an additional error ((guid, pid int64, err error))?
Alternatively, you could set the error return value to -1, -1.
This is because it is guaranteed that no process will ever have a negative PID, while it could happen that a process has PID 0, although that is close to impossible for a process that is not the OS.

routers/repo/migrate.go

delvh · 2021-09-28T14:22:16Z

routers/repo/migrate.go

+		log.Trace("Migration [%s] cancel PID [%d] ", ctx.Repo.Repository.FullName(), tPID)
+		process.GetManager().Cancel(tPID)
+		ctx.Flash.Success(ctx.Tr("repo.migrate.cancelled"))
+		ctx.Redirect(ctx.Repo.Owner.DashboardLink())


Is it even sensible to flash a message if the next action is to redirect back to the homepage?

delvh · 2021-09-28T14:22:26Z

routers/repo/migrate.go

+		return
+	}
+
+	ctx.Flash.Error(ctx.Tr("repo.migrate.task_not_found", ctx.Repo.Repository.FullName()))


Is it even sensible to flash a message if the next action is to redirect back to the homepage?

delvh · 2021-09-28T14:26:24Z

routers/repo/migrate.go

+	tGUID, tPID := task.GUIDandPID()
+	if guid == tGUID {
+		log.Trace("Migration [%s] cancel PID [%d] ", ctx.Repo.Repository.FullName(), tPID)
+		process.GetManager().Cancel(tPID)


I think it would be best to delete the repository here explicitly.
Otherwise, the repo might or might not exist in the database,
resulting in the user being unable to recreate a repo under this name until
either an admin runs a health check, or a doctor command is scheduled.

Co-authored-by: delvh <dev.lh@web.de>

singuliere · 2022-02-04T13:55:07Z

This is an interesting feature, how could the conflicts be resolved? Would you like to rebase or should it be done in a different way?

6543 · 2022-02-04T16:14:11Z

I'll resolve the conflicts ...

6543 · 2022-02-08T16:40:14Z

to explain the problem why it did not got merged jet: if you have a setup with more instances, and you like to cancle a migration from instance A running on instance B, it wont work (A will find the job in the database but cant find it in it's process.Manager)

wxiaoguang · 2023-05-09T13:08:43Z

Replaced by Make repo migration cancelable and fix various bugs #24605

Replace #12917 Close #24601 Close #12845 ![image](https://github.com/go-gitea/gitea/assets/2114189/39378118-064d-40fb-8396-4579ebf33917) ![image](https://github.com/go-gitea/gitea/assets/2114189/faf37418-191c-46a6-90a8-353141e00e2d) ![image](https://github.com/go-gitea/gitea/assets/2114189/fdc8ee4d-125f-4737-9990-89bcdf9eb388) ![image](https://github.com/go-gitea/gitea/assets/2114189/9a3bd2c2-fe20-4011-81f0-990ed869d139) --------- Co-authored-by: Yarden Shoham <git@yardenshoham.com> Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: Giteabot <teabot@gitea.io>

6543 mentioned this pull request Sep 21, 2020

-deleted- #12904

Closed

6543 added type/enhancement An improvement of existing functionality topic/ui Change the appearance of the Gitea UI pr/wip This PR is not ready for review labels Sep 21, 2020

6543 added this to the 1.14.0 milestone Sep 21, 2020

6543 commented Sep 21, 2020

View reviewed changes

routers/repo/migrate.go Show resolved Hide resolved

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 21, 2020

6543 mentioned this pull request Sep 21, 2020

Migration: add Cancel button #12845

Closed

zeripath changed the title ~~Add Cancle button for Migration~~ Add Cancel button for Migration Oct 8, 2020

silverwind reviewed Oct 14, 2020

View reviewed changes

templates/repo/migrate/migrating.tmpl Outdated Show resolved Hide resolved

silverwind reviewed Oct 14, 2020

View reviewed changes

templates/repo/migrate/migrating.tmpl Outdated Show resolved Hide resolved

silverwind reviewed Oct 14, 2020

View reviewed changes

routers/repo/migrate.go Outdated Show resolved Hide resolved

silverwind reviewed Oct 14, 2020

View reviewed changes

routers/routes/routes.go Outdated Show resolved Hide resolved

6543 commented Oct 15, 2020

View reviewed changes

templates/repo/migrate/migrating.tmpl Outdated Show resolved Hide resolved

routers/routes/routes.go Outdated Show resolved Hide resolved

6543 force-pushed the ui-migration-cancel branch from a7f8c55 to 2fd3802 Compare October 27, 2020 19:28

6543 commented Nov 28, 2020

View reviewed changes

routers/repo/migrate.go Show resolved Hide resolved

6543 removed the pr/wip This PR is not ready for review label Nov 28, 2020

silverwind reviewed Nov 29, 2020

View reviewed changes

routers/repo/migrate.go Outdated Show resolved Hide resolved

silverwind approved these changes Nov 29, 2020

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 29, 2020

6543 added topic/repo-migration Migrate repos from other platforms to Gitea, or from Gitea to them and removed topic/ui Change the appearance of the Gitea UI labels Dec 1, 2020

6543 added 2 commits December 2, 2020 19:57

Merge branch 'master' into ui-migration-cancel

f7e9022

Merge branch 'master' into ui-migration-cancel

cfed3be

lunny modified the milestones: 1.14.0, 1.15.0 Jan 27, 2021

lunny mentioned this pull request Mar 23, 2021

Cancel mirror process not possible #15108

Closed

6 tasks

techknowlogick modified the milestones: 1.15.0, 1.16.0 Jun 15, 2021

delvh mentioned this pull request Sep 10, 2021

Neverending Repository Migration Blocks All Current and Future Migrations #17005

Closed

1 task

delvh reviewed Sep 28, 2021

View reviewed changes

lunny modified the milestones: 1.16.0, 1.17.0 Nov 9, 2021

Update routers/repo/migrate.go

b5c2461

Co-authored-by: delvh <dev.lh@web.de>

6543 self-assigned this Feb 8, 2022

lunny modified the milestones: 1.17.0, 1.18.0 May 25, 2022

lunny modified the milestones: 1.18.0, 1.19.0 Oct 17, 2022

lunny modified the milestones: 1.19.0, 1.20.0 Jan 31, 2023

wxiaoguang mentioned this pull request May 9, 2023

Make repo migration cancelable and fix various bugs #24605

Merged

wxiaoguang closed this May 9, 2023

GiteaBot removed this from the 1.20.0 milestone May 9, 2023

6543 deleted the ui-migration-cancel branch July 18, 2023 13:24

6543 removed their assignment Jul 18, 2023

go-gitea locked as resolved and limited conversation to collaborators Aug 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Migrations Cancelable #12917

Make Migrations Cancelable #12917

6543 commented Sep 21, 2020 •

edited

Loading

codecov-commenter commented Sep 21, 2020 •

edited

Loading

codecov-io commented Oct 7, 2020 •

edited

Loading

6543 commented Nov 28, 2020

silverwind commented Nov 28, 2020

6543 commented Nov 28, 2020

silverwind commented Nov 28, 2020 •

edited

Loading

6543 commented Nov 28, 2020

delvh Sep 28, 2021

delvh Sep 28, 2021

delvh Sep 28, 2021

6543 Sep 28, 2021

delvh Sep 28, 2021

delvh Sep 28, 2021

delvh Sep 28, 2021

delvh Sep 28, 2021

singuliere commented Feb 4, 2022

6543 commented Feb 4, 2022

6543 commented Feb 8, 2022

wxiaoguang commented May 9, 2023

		@@ -0,0 +1,17 @@
		// Copyright 2020 The Gitea Authors. All rights reserved.

Make Migrations Cancelable #12917

Make Migrations Cancelable #12917

Conversation

6543 commented Sep 21, 2020 • edited Loading

codecov-commenter commented Sep 21, 2020 • edited Loading

Codecov Report

codecov-io commented Oct 7, 2020 • edited Loading

Codecov Report

6543 commented Nov 28, 2020

silverwind commented Nov 28, 2020

6543 commented Nov 28, 2020

silverwind commented Nov 28, 2020 • edited Loading

6543 commented Nov 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

singuliere commented Feb 4, 2022

6543 commented Feb 4, 2022

6543 commented Feb 8, 2022

wxiaoguang commented May 9, 2023

6543 commented Sep 21, 2020 •

edited

Loading

codecov-commenter commented Sep 21, 2020 •

edited

Loading

codecov-io commented Oct 7, 2020 •

edited

Loading

silverwind commented Nov 28, 2020 •

edited

Loading