The Wayback Machine - https://web.archive.org/web/20221022163541/https://github.com/go-gitea/gitea/pull/17940
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

Jump to bottom

Decouple HookTask from Repository #17940

Merged
merged 24 commits into from Oct 21, 2022
Merged

Decouple HookTask from Repository #17940

merged 24 commits into from Oct 21, 2022
+216 −279

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Dec 9, 2021 •
edited

At the moment a repository reference is needed for webhooks. With the upcoming package PR we need to send webhooks without a repository reference. For example a package is uploaded to an organization. In theory this enables the usage of webhooks for future user actions.

This PR removes the repository id from HookTask and changes how the hooks are processed (see services/webhook/deliver.go). In a follow up PR I want to remove the usage of the `UniqueQueue´ and replace it with a normal queue because there is no reason to be unique.

lunny
lunny previously requested changes Dec 10, 2021
View changes
models/repo.go Show resolved Hide resolved
models/webhook/hooktask.go Show resolved Hide resolved
This was referenced Dec 14, 2021
Open
Open
@lunny
Copy link
Member

lunny commented Dec 18, 2021

How about to add a new column named type which could be respository or package or user, and the repo_id could be changed to target_id?

lunny
lunny reviewed Feb 18, 2022
View changes
models/repo.go Show resolved Hide resolved
@lunny lunny dismissed their stale review Feb 18, 2022

It should not be a block.

@singuliere
Copy link
Contributor

singuliere commented Feb 22, 2022

@KN4CK3R could you please resolve the conflict?

This was referenced Mar 28, 2022
Merged
Open
wxiaoguang
wxiaoguang reviewed Mar 31, 2022
View changes
models/webhook/hooktask.go Outdated Show resolved Hide resolved
services/webhook/webhook.go Outdated Show resolved Hide resolved
services/webhook/webhook.go Outdated Show resolved Hide resolved
@6543 6543 added this to the 1.17.0 milestone May 3, 2022
6543
6543 reviewed May 12, 2022
View changes
models/repo.go Outdated Show resolved Hide resolved
6543
6543 reviewed May 12, 2022
View changes
models/webhook/hooktask.go Outdated Show resolved Hide resolved
6543
6543 reviewed May 12, 2022
View changes
services/webhook/webhook.go Outdated Show resolved Hide resolved
@lunny
Copy link
Member

lunny commented Jun 3, 2022

Please resolve the conflicts

@lunny lunny modified the milestones: 1.17.0, 1.18.0 Jun 16, 2022
@KN4CK3R KN4CK3R mentioned this pull request Aug 8, 2022
Open
@6543
Copy link
Member

6543 commented Oct 17, 2022

@KN4CK3R please resolve conflicts

@6543
Copy link
Member

6543 commented Oct 19, 2022

https://github.com/go-gitea/gitea/pull/17940/files#r871602755

@lunny
Copy link
Member

lunny commented Oct 21, 2022 •
edited

Maybe I have posted this concern. When a hook task created from an org/system level webhook, if repo_id is removed from table hook_task, we cannot know which repository the hook_task belongs to easily.

@6543
Copy link
Member

6543 commented Oct 21, 2022

@lunny we do store RepoID in Webhook table: https://github.com/go-gitea/gitea/pull/17940/files#diff-8fd01247d233d732c2e8ffc0c554ad22dde28f35fc7e27f5489d5da39f53c79bR184

6543
6543 approved these changes Oct 21, 2022
View changes
@6543
Copy link
Member

6543 commented Oct 21, 2022

gitea/models/webhook/webhook.go

Line 190 in 716fcfc

RepoID int64 `xorm:"INDEX"` // An ID of 0 indicates either a default or system webhook

@6543 6543 merged commit 1887c95 into go-gitea:main Oct 21, 2022
2 checks passed
@lunny
Copy link
Member

lunny commented Oct 22, 2022 •
edited

@lunny we do store RepoID in Webhook table: https://github.com/go-gitea/gitea/pull/17940/files#diff-8fd01247d233d732c2e8ffc0c554ad22dde28f35fc7e27f5489d5da39f53c79bR184

You don't really understand how webhook and hooktask work. A system/org level webhook's repo_id is always zero.

@KN4CK3R KN4CK3R deleted the feature-user-org-webhook branch Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lunny lunny

@wxiaoguang wxiaoguang

@silverwind silverwind

@6543 6543

@singuliere singuliere

Assignees
No one assigned
Labels
kind/enhancement lgtm/done
Milestone
1.18.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@KN4CK3R @lunny @singuliere @6543 @silverwind @wxiaoguang @GiteaBot