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

PUSH event hooks not sent for branch removal #2105

Closed
1 of 7 tasks
stephenc opened this issue Jul 3, 2017 · 17 comments
Closed
1 of 7 tasks

PUSH event hooks not sent for branch removal #2105

stephenc opened this issue Jul 3, 2017 · 17 comments
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Milestone

Comments

@stephenc
Copy link

stephenc commented Jul 3, 2017

  • Gitea version (or commit ref): 858197b
  • Git version: N/A
  • Operating system: N/A
  • Database (use [x]): N/A
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

When I configure a webhook that receives all events, push events that remove a branch or branch removal of the branch through the UI do not trigger a webhook event:

Creation of a new branch results in events like:

X-Gitea-Event: create

{
  "secret": "magic",
  "sha": "2409990415d77f47ed5c4c775abda4ffa6aa5f11",
  "ref": "branch",
  "ref_type": "branch",
  "repository": {
    "id": 17,
    "owner": {
...

and

X-Gitea-Event: push

{
  "secret": "magic",
  "ref": "refs/heads/branch",
  "before": "0000000000000000000000000000000000000000",
  "after": "2409990415d77f47ed5c4c775abda4ffa6aa5f11",
...

I would expect removal of the branch to trigger at least a webhook that looked something like:

X-Gitea-Event: push

{
  "secret": "magic",
  "ref": "refs/heads/branch",
  "before": "2409990415d77f47ed5c4c775abda4ffa6aa5f11",
  "after": "0000000000000000000000000000000000000000",
...

It is a somewhat separate concern the use of a magic value to indicate no ref, better would be to include the created and deleted booleans that GitHub does, but for my part the only concern I have is the lack of a push event to catch branch removal.

@bkcsoft
Copy link
Member

bkcsoft commented Jul 3, 2017

Could you include the logs from gitea? without them it's very hard to see what's happening :)

@bkcsoft bkcsoft removed the type/bug label Jul 3, 2017
@bkcsoft
Copy link
Member

bkcsoft commented Jul 3, 2017

(Not sure if this is bug or not, might be a feature request 😅 )

@stephenc
Copy link
Author

stephenc commented Jul 3, 2017

I'll see if I can dig it out but your Jenkins integration (which I have nearly finished) will be annoying without an event for this.

If you want feature requests:

  • manage hooks for a user's repos
  • events for repo created / deleted
  • some way in the api to distinguish a name as an org name vs user name (currently I get the name as a user and it's and org if the email is empty)
  • file browse by rest api (basically the raw needs a way to serve directory listings for non-file paths... also good to flag content as a link if somebody commits a link)

Icing on top:

  • commit file (with branch creation if desired) by rest api - needed for blue ocean integration

@stephenc
Copy link
Author

stephenc commented Jul 3, 2017

Logs

Creating the branch: (git push origin branch)

Jul  3 20:05:56 sshd[27634]: Accepted publickey for git from x.x.x.x port 58096 ssh2: RSA SHA256:BTv+kWSXkHaRXdqhCNGet61YTXyMvWnfOFfSWr3Mgr8
[Macaron] 2017-07-03 20:05:56: Started GET /api/internal/branch/17/branch for 127.0.0.1
[Macaron] 2017-07-03 20:05:56: Completed /api/internal/branch/17/branch 200 OK in 2.354313ms
[Macaron] 2017-07-03 20:05:56: Started POST /api/internal/push/update for 127.0.0.1
[Macaron] 2017-07-03 20:05:56: Completed /api/internal/push/update 202 Accepted in 61.8573ms
[Macaron] 2017-07-03 20:05:56: Started POST /api/internal/ssh/1/update for 127.0.0.1
[Macaron] 2017-07-03 20:05:56: Completed /api/internal/ssh/1/update 200 OK in 13.464549ms
Jul  3 20:05:56 sshd[27634]: Received disconnect from x.x.x.x port 58096:11: disconnected by user
Jul  3 20:05:56 sshd[27634]: Disconnected from x.x.x.x port 58096

Events from the UI
screen shot 2017-07-03 at 21 07 07
screen shot 2017-07-03 at 21 07 19

Now the log when deleting the branch (git push origin :branch)

Jul  3 20:05:59 sshd[27690]: Accepted publickey for git from x.x.x.x port 58098 ssh2: RSA SHA256:BTv+kWSXkHaRXdqhCNGet61YTXyMvWnfOFfSWr3Mgr8
[Macaron] 2017-07-03 20:05:59: Started GET /api/internal/branch/17/branch for 127.0.0.1
[Macaron] 2017-07-03 20:05:59: Completed /api/internal/branch/17/branch 200 OK in 2.063422ms
[Macaron] 2017-07-03 20:05:59: Started POST /api/internal/push/update for 127.0.0.1
[Macaron] 2017-07-03 20:05:59: Completed /api/internal/push/update 202 Accepted in 9.581596ms
[Macaron] 2017-07-03 20:05:59: Started POST /api/internal/ssh/1/update for 127.0.0.1
[Macaron] 2017-07-03 20:05:59: Completed /api/internal/ssh/1/update 200 OK in 4.247786ms
Jul  3 20:05:59 sshd[27690]: Received disconnect from x.x.x.x port 58098:11: disconnected by user
Jul  3 20:05:59 sshd[27690]: Disconnected from x.x.x.x port 58098

No events in the UI, no events in request bin

@bkcsoft does that help?

@lunny lunny added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Jul 4, 2017
@lunny lunny added this to the 1.3.0 milestone Jul 4, 2017
@lunny
Copy link
Member

lunny commented Jul 4, 2017

I think that's a feature, see the description on the webhook on Gitea. There is no delete-branch.
-2

@stephenc
Copy link
Author

stephenc commented Jul 4, 2017

Says "git push to a repository" so should be all git pushes, even those that remove refs.

That's a bug if you ask me.

A feature would be a separate new event for "delete" but the "push" should include removal based on its description

@stephenc
Copy link
Author

stephenc commented Jul 4, 2017

@lunny
screen shot 2017-07-04 at 14 52 14

Just says Git push to a repository it doesn't say Subset of git pushes to a repository that add or update things but not those that remove things

This is clearly a bug

@stephenc
Copy link
Author

stephenc commented Jul 4, 2017

If you want to have a new event for non-push that is a delete event, that would be a separate feature request, but this issue is PUSH event hooks not sent for branch removal

@bkcsoft
Copy link
Member

bkcsoft commented Jul 4, 2017

I would separate these checkboxes into categories, so you get somethings like this. WDYT?

  • Git
    • Push event (new commits)
    • Branch events (create, delete)
    • Tag events (create, delete)
  • PullRequest
    • Opened
    • Closed
    • [...]
  • Issues
    • Opened
    • [...]

@stephenc
Copy link
Author

stephenc commented Jul 4, 2017

I think push should align with the GitHub push event, i.e. Branch/tag CRUD

The other stuff would be a great feature request, but missing the D on this is a bug

@stephenc
Copy link
Author

stephenc commented Jul 4, 2017

I also think that too many options is bad for users. GitHub has it about right:

screen shot 2017-07-04 at 22 13 51

You just need to fix this bug on how you process Push events... which should include pushes like git push origin :branch just like GitHub does.

The PUSH events currently are missing, clear bug.

The other events that GitHub offers are RFEs and should probably be in a separate issue

@stephenc
Copy link
Author

stephenc commented Jul 4, 2017

@lunny @bkcsoft I have created #2113 to track the RFEs so that this one can be left to track the BUG ;-)

@bkcsoft
Copy link
Member

bkcsoft commented Jul 5, 2017

If a hook is missing that never was there in the first place it's still a FE, not a bug ;)

@bkcsoft bkcsoft removed the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Jul 5, 2017
@stephenc
Copy link
Author

stephenc commented Jul 5, 2017

Well you already have a "Push" hook that promises "Git push to a repository" and I am showing that it does not deliver all git pushes to a repository... only the Cr & U of CrUD... so sounds very like a bug.... I am not asking for a new hook in this issue... only for the existing hook to publish all the "git push to a repository" that it currently advertises

Now you could fix this in one of two ways:

  1. Actually fix the existing hook to honour its existing documentation
  2. Change the documentation to say that "push" does not include the D in CrUD

Either of those are perfectly valid fixes to the bug... obviously I would be a Sad Panda if you chose the "change the documentation" route.

BTW when you fix this then https://github.com/jenkinsci/gitea-plugin will be a really sweet integration for gitea with Jenkins... right now, without the Push event publishing the D of CrUD you have to force scanning far far too often in order to clear out deleted branches

@lunny lunny mentioned this issue Sep 1, 2017
6 tasks
@daviian
Copy link
Member

daviian commented Sep 14, 2017

Someone's working on that?
Otherwise I would like to implement that.

@lafriks
Copy link
Member

lafriks commented Sep 14, 2017

@daviian I don't think so, go ahead

@daviian
Copy link
Member

daviian commented Sep 21, 2017

IMO issue can be closed now as #2530 has been merged

@lunny lunny closed this as completed Sep 22, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

No branches or pull requests

5 participants