search

deis / builder

Git server and application builder for Deis Workflow
https://deis.com
MIT License
40 stars 41 forks source link

ref(gitreceive): remove push hook #407

Closed Joshua-Anderson closed 8 years ago

Joshua-Anderson commented 8 years ago

It was used for authorization, but the keys hooks is now used for both authentication and authorization.

deis-bot commented 8 years ago

@arschles, @aledbf and @kmala are potential reviewers of this pull request based on my analysis of git blame information. Thanks @Joshua-Anderson!

codecov-io commented 8 years ago

Current coverage is 46.48% (diff: 0.00%)

Merging #407 into master will decrease coverage by 0.14%

@@             master       #407   diff @@
==========================================
  Files            26         26          
  Lines          1098       1095     -3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            512        509     -3   
  Misses          555        555          
  Partials         31         31

Powered by Codecov. Last update 0690f89...d50cb23

helgi commented 8 years ago

We'd not have the push audit log anymore, not that it is exposed anywhere right now

Joshua-Anderson commented 8 years ago

Right now the push audit log isn't exposed anywhere. Is it worth exposing the push audit log and keeping the hook or getting rid of the audit log and the hook?

helgi commented 8 years ago

@gabrtv, @mboersma and @bacongobbler would have some context here

Overall I'm fine with removing the hook - A different audit log approach can also be introduced back in the future

bacongobbler commented 8 years ago

Is it worth exposing the push audit log and keeping the hook or getting rid of the audit log and the hook?

what exactly are you referring to when you mean "the push audit log"? Haven't heard that terminology used before.

Joshua-Anderson commented 8 years ago

All this hook does is add the fields of the request to a table in the DB. None of our code actualy uses the table though.

Joshua-Anderson commented 8 years ago

This is the push model

bacongobbler commented 8 years ago

ah I see. Yes, server side it's just validation. Nothing is done with the request.

smothiki commented 8 years ago

@bacongobbler @mboersma I dont have much context with Push Hook can you guys take a quick look or we can punt this to the next sprint .