-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Prune hook_task table #11416
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
Prune hook_task table #11416
Changes from all commits
e1c9664
ee83fe7
c4502dc
739dfe7
2cdecc1
dd5a870
dfb2505
1e800f5
2ae404b
c016b7c
eea3a74
1a56206
66c0d8b
fde6937
dd0f4a1
bc14884
8a66680
125dc99
d1db4f2
fd13812
a28df99
4651030
aec78da
38f6d30
2170896
537c913
b6c09c5
ce8d93a
3c45159
28c73de
a803bf2
2c76c5f
4a83485
9b2b546
a4caeca
83925c2
5decea3
9a09960
6f9af14
49ea492
deaac3e
1709bc9
7ebc11c
157fd96
dacb03b
d1642ac
63551dd
0341fc6
cffae2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
// Copyright 2020 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package migrations | ||
|
||
import ( | ||
"code.gitea.io/gitea/modules/setting" | ||
|
||
"xorm.io/xorm" | ||
) | ||
|
||
func addHookTaskPurge(x *xorm.Engine) error { | ||
type Repository struct { | ||
ID int64 `xorm:"pk autoincr"` | ||
IsHookTaskPurgeEnabled bool `xorm:"NOT NULL DEFAULT true"` | ||
NumberWebhookDeliveriesToKeep int64 `xorm:"NOT NULL DEFAULT 10"` | ||
} | ||
|
||
if err := x.Sync2(new(Repository)); err != nil { | ||
return err | ||
} | ||
|
||
_, err := x.Exec("UPDATE repository SET is_hook_task_purge_enabled = ?, number_webhook_deliveries_to_keep = ?", | ||
setting.Repository.DefaultIsHookTaskPurgeEnabled, setting.Repository.DefaultNumberWebhookDeliveriesToKeep) | ||
return err | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ package models | |
import ( | ||
"encoding/json" | ||
"testing" | ||
"time" | ||
|
||
api "code.gitea.io/gitea/modules/structs" | ||
|
||
|
@@ -245,3 +246,58 @@ func TestUpdateHookTask(t *testing.T) { | |
assert.NoError(t, UpdateHookTask(hook)) | ||
AssertExistsAndLoadBean(t, hook) | ||
} | ||
|
||
func TestDeleteDeliveredHookTasks_DeletesDelivered(t *testing.T) { | ||
assert.NoError(t, PrepareTestDatabase()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you insert 2 and limit to 1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, I added a test where I created a new webhook that was delievered "now" and then called the delete method where 1 is kept...and then verified that the newly created webhook is still present. |
||
hookTask := &HookTask{ | ||
RepoID: 3, | ||
HookID: 3, | ||
Type: GITEA, | ||
URL: "http://www.example.com/unit_test", | ||
Payloader: &api.PushPayload{}, | ||
IsDelivered: true, | ||
} | ||
AssertNotExistsBean(t, hookTask) | ||
assert.NoError(t, CreateHookTask(hookTask)) | ||
AssertExistsAndLoadBean(t, hookTask) | ||
|
||
assert.NoError(t, DeleteDeliveredHookTasks(3, 0)) | ||
AssertNotExistsBean(t, hookTask) | ||
} | ||
|
||
func TestDeleteDeliveredHookTasks_LeavesUndelivered(t *testing.T) { | ||
assert.NoError(t, PrepareTestDatabase()) | ||
hookTask := &HookTask{ | ||
RepoID: 2, | ||
HookID: 4, | ||
Type: GITEA, | ||
URL: "http://www.example.com/unit_test", | ||
Payloader: &api.PushPayload{}, | ||
IsDelivered: false, | ||
} | ||
AssertNotExistsBean(t, hookTask) | ||
assert.NoError(t, CreateHookTask(hookTask)) | ||
AssertExistsAndLoadBean(t, hookTask) | ||
|
||
assert.NoError(t, DeleteDeliveredHookTasks(3, 0)) | ||
AssertExistsAndLoadBean(t, hookTask) | ||
} | ||
|
||
func TestDeleteDeliveredHookTasks_LeavesMostRecentTask(t *testing.T) { | ||
assert.NoError(t, PrepareTestDatabase()) | ||
hookTask := &HookTask{ | ||
RepoID: 2, | ||
HookID: 4, | ||
Type: GITEA, | ||
URL: "http://www.example.com/unit_test", | ||
Payloader: &api.PushPayload{}, | ||
IsDelivered: true, | ||
Delivered: time.Now().UnixNano(), | ||
} | ||
AssertNotExistsBean(t, hookTask) | ||
assert.NoError(t, CreateHookTask(hookTask)) | ||
AssertExistsAndLoadBean(t, hookTask) | ||
|
||
assert.NoError(t, DeleteDeliveredHookTasks(3, 1)) | ||
AssertExistsAndLoadBean(t, hookTask) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,8 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (_ *m | |
IsPrivate: opts.IsPrivate, | ||
IsFsckEnabled: !opts.IsMirror, | ||
CloseIssuesViaCommitInAnyBranch: setting.Repository.DefaultCloseIssuesViaCommitsInAnyBranch, | ||
IsHookTaskPurgeEnabled: setting.Repository.DefaultIsHookTaskPurgeEnabled, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we real have to set it on repo creation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I was implementing the way "CloseIssuesViaCommitInAnyBranch" is done...but I think you are suggesting that the columns added to repository would be null by default, and would only have values if the user overrides the settings? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes |
||
NumberWebhookDeliveriesToKeep: setting.Repository.DefaultNumberWebhookDeliveriesToKeep, | ||
Status: opts.Status, | ||
IsEmpty: !opts.AutoInit, | ||
TrustModel: opts.TrustModel, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
// Copyright 2020 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package repository | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"code.gitea.io/gitea/models" | ||
"code.gitea.io/gitea/modules/log" | ||
|
||
"xorm.io/builder" | ||
zeripath marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
// PruneHookTaskTable deletes rows from hook_task as needed. | ||
func PruneHookTaskTable(ctx context.Context) error { | ||
log.Trace("Doing: PruneHookTaskTable") | ||
|
||
if err := models.Iterate( | ||
models.DefaultDBContext(), | ||
new(models.Repository), | ||
builder.Expr("id>0 AND is_hook_task_purge_enabled=?", true), | ||
func(idx int, bean interface{}) error { | ||
select { | ||
case <-ctx.Done(): | ||
return fmt.Errorf("Aborted due to shutdown") | ||
default: | ||
} | ||
repo := bean.(*models.Repository) | ||
repoPath := repo.RepoPath() | ||
log.Trace("Running prune hook_task table on repository %s", repoPath) | ||
if err := models.DeleteDeliveredHookTasks(repo.ID, repo.NumberWebhookDeliveriesToKeep); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may fail on sqlite. I think use a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I totally understand, can you elaborate? I saw Iterate usage other places like for example (if I'm understanding): But I'm happy to refactor it if it is a problem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When did some database operations on an iterate maybe affect a lock on sqlite. If other places also have the problems, they also needs to be changed. |
||
desc := fmt.Sprintf("Failed to prune hook_task on repository (%s): %v", repoPath, err) | ||
log.Warn(desc) | ||
if err = models.CreateRepositoryNotice(desc); err != nil { | ||
log.Error("CreateRepositoryNotice: %v", err) | ||
} | ||
} | ||
return nil | ||
}, | ||
); err != nil { | ||
return err | ||
} | ||
|
||
log.Trace("Finished: PruneHookTaskTable") | ||
return nil | ||
} |
Uh oh!
There was an error while loading. Please reload this page.