Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Webhooks for yanked gems #366

Closed
wants to merge 1,853 commits into
from

Conversation

Projects
None yet
Contributor

laserlemon commented Nov 21, 2011

Yanking gems is a necessary evil.

I don't think there's any good way to prevent authors from prematurely pushing broken or unfinished gems. Unfortunately, yanked gems often leave people's projects in gem dependency limbo. For instance, the good people of @collectiveidea have found their projects depending upon the gherkin gem (indirectly through cucumber) and the surrounding dependencies are such that gherkin is locked down to ~> 2.3.8. The problem is that the only versions that satisfy that requirement are 2.3.8, 2.3.9 and 2.3.10, all of which have been yanked.

http://rubygems.org/gems/gherkin/versions

While scenarios like the one above can't always be avoided, it's important to notify developers when a version is yanked so they can take appropriate actions and I think that the existing web hook functionality is a great way to deliver those notifications.

In my mind, the easiest way to pull it off is to hit the same registered web hook URLs rather than registering separate hooks for yanks. So if we were to hit the same URLs, either the method or the payload would have to change in order to differentiate pushes and yanks. I was in favor of an identical DELETE request until I started getting mixed signals on whether DELETE requests can/should have request bodies.

If we were to change the web hook payload, I think the existing webhook bodies should stay the same and the new hooks should take on a backwards-incompatible form so that yanked gem versions aren't mistaken for newly pushed gem versions with existing web hook subscribers.

What are your feelings on the concept of yank web hooks and their implementation?

sferik and others added some commits Sep 3, 2011

@sferik sferik Update gravtastic dependency to version 3.2.6 cd4ca6b
@sferik sferik Update rdoc dependency to version 3.9.4 cfe1943
@sferik sferik Update webmock dependency to version 1.7.6 365364c
@sferik sferik Update capybara dependency to version 1.1.1 0c01aaa
@cmeiklejohn cmeiklejohn Add a migration to drop some unused columns [#333] 0a77910
@cldwalker cldwalker add tests for dependencies api endpoint 185fd65
@sferik sferik Update gherkin dependency to 2.4.18, mostly just to force a Travis build
This should be a low-risk development dependency update.
0433ff4
@sferik sferik Don't test against Ruby 1.9.2 7ac18b1
@sferik sferik Update excon dependency to version 0.6.6 2d1a285
@sferik sferik Update high_voltage dependency to version 1.0.1 9ba3150
@sferik sferik Update newrelic_rpm dependency to version 3.1.2 884ee11
@sferik sferik Update method_source dependency to version 0.6.5 bb29c6c
@sferik sferik Update ruby_parser dependency to version 2.3.0 d7482d0
@sferik sferik Update pry dependency to version 0.9.5 28cc48a
@sferik sferik Update cucumber_rails dependency to version 1.0.3 bb8f7e5
@sferik sferik Manually start Redis before running tests on Travis e3e0254
@sferik sferik Revert "Manually start Redis before running tests on Travis"
This reverts commit 2146357.
34f68cd
@sferik sferik Update cucumber dependency to version 1.0.6
Progress against issue #345
37b1391
@sferik sferik Downgrade cucumber-rails dependency to version 1.0.2
Fixes #345
af313a0
@nickrivadeneira nickrivadeneira get rid of the rubygems.org gemset, this is silly 311c047
@nickrivadeneira nickrivadeneira use factory_girl 2.0 DSL for parent factories 1a677e0
@nickrivadeneira nickrivadeneira set the yamler to psych in initializer too a8b1238
@nickrivadeneira nickrivadeneira apparently I am inept at factory girl 20b321e
@sferik sferik Update cucumber-rails dependency to version 1.0.5.
Making an exception to the mid-week update policy in order to resolve
the YAML::Syck::DefaultKey issue.
b1ca7ed
@sferik sferik Update clearance dependency to version 0.12.0 d71e74a
@sferik sferik Remove confirmed_password d449ab1
@sferik sferik Replace Factory(:email_confirmed_user) with Factory(:user)
See f2f3a1e9ec4a72b7164f90038531dbfd17200b91.
45cad6e
@sferik sferik Generate new clearance features be47da8
@sferik sferik Remove confirmations controller 35a94b9
@sferik sferik Make unit tests pass with new clearance version 730023c
@cmeiklejohn @sferik cmeiklejohn + sferik Require clearance/testing instead of clearance/shoulda_macros. c827f1c
@cmeiklejohn @sferik cmeiklejohn + sferik There is no more email confirmed. 1b8cff6
@cmeiklejohn @sferik cmeiklejohn + sferik No longer test for confirmed users. 9af25c6
@cmeiklejohn @sferik cmeiklejohn + sferik No longer confirm email. e728b11
@cmeiklejohn @sferik cmeiklejohn + sferik No longer email_confirmed? 17147fe
@cmeiklejohn @sferik cmeiklejohn + sferik Update features with new clearance syntax. baab17b
@cmeiklejohn @sferik cmeiklejohn + sferik Update clearance syntax. 573b260
@cmeiklejohn @sferik cmeiklejohn + sferik Update clearance step to assign current user to @me. ad3efb8
@cmeiklejohn @sferik cmeiklejohn + sferik Fix missed syntax change. aafa9f8
@cmeiklejohn @sferik cmeiklejohn + sferik There is no longer a flash success after create in clearance. dc31622
@cmeiklejohn @sferik cmeiklejohn + sferik No longer a flash message on successful sign-in. bedf403
@cmeiklejohn @sferik cmeiklejohn + sferik We no longer have email confirmed users. 0d6d187
@cmeiklejohn @sferik cmeiklejohn + sferik One error message on too-long, but well-formed email; no confirmation…
… after signup.
aeb2ad4
@smoak smoak Adding api support for most downloaded of all time. 66808fb
@smoak smoak Added unit tests for changes to Download 51a5c4b
@michaelfairley michaelfairley Have api/v1/gems/just_updated return gems rather than versions 2c32cb3
@cmeiklejohn cmeiklejohn Don't require that the sign in and sign out links be in title case. deb89c7
@cmeiklejohn cmeiklejohn Only one error message here. f496719
@cmeiklejohn cmeiklejohn Fix some more testing against case insensitive links. 3f7fa84
@cmeiklejohn cmeiklejohn Fix form localization. 479c88c
@cmeiklejohn cmeiklejohn DOn't require that the sign in check be on a particular page. c6ce824
@cmeiklejohn cmeiklejohn Add guard. 0aa624c
@cmeiklejohn cmeiklejohn Add guard-bundler. f4edaad
@cmeiklejohn cmeiklejohn The app has a capybara dependency for testing through the clearance f…
…eatures.
5802231
@cmeiklejohn cmeiklejohn Hard code the steps of singing in with a new password, since we only …
…ever do it once in all of the scenarios.
c87a663
@cmeiklejohn cmeiklejohn Remove session is cleared code. d878678
@cmeiklejohn cmeiklejohn Fix selector syntax issue in the clearance steps. b9362ac
@cmeiklejohn cmeiklejohn Downcase the email in the override, as stock clearance would. 0219bf0
@cmeiklejohn cmeiklejohn Update stock clearance matcher for email field to match our email or …
…username fields.
8d235e2
@cmeiklejohn cmeiklejohn Add home page path. ac6e31b

sferik and others added some commits Nov 26, 2011

@sferik sferik Update jquery-rails dependency to version 1.0.19 3dbd9c9
@sferik sferik Update rubyzip dependency to version 0.9.5 52ebfc3
@sferik sferik Update factory_girl dependency to version 2.3.2 5eca100
@sferik sferik Remove psych gem (not necessary in Ruby 1.9) 50d823c
@sferik sferik Update childprocess dependency to version 0.2.3 c489cf5
@sferik sferik Rubinius 2.0 is now the default Rubinius on Travis 92690c6
@sferik sferik Update json dependency to version 1.6.2 67dd4fc
@qrush qrush lets try not locking to syck ccae0bc
@sferik sferik Update multi_json dependency to version 1.0.4 ab4065f
@qrush qrush Trust rvmrc everywhere 94a2865
@qrush qrush just trust rvmrcs in /etc/rvmrc instead b8c5f9c
@sferik sferik Update selenium-webdriver dependency to version 2.14.0 cfec77f
@sferik sferik Update delayed_job dependency to version 3.0.0.pre4 fa974d3
@sferik sferik Replace validates_url_format_of with validates_formatting_of
The validates_url_format_of gem was poorly maintained. I know, because I
help maintain it. :P
30eb2ee
@sferik sferik Update excon dependency to version 0.7.9 145a030
@sferik sferik Update json dependency to version 1.6.3 023a4bd
@sferik sferik Convert to Markdown 2f8fbbe
@sferik sferik Fix broken images [ci skip] 7cc6f58
@sferik sferik Fix broken link [ci skip] bb24ef9
@sferik sferik Update validates_formatting_of dependency to version 0.3.1 630185a
@sferik sferik Run tests on 1.9.2 34c0d5e
@sferik sferik Run tests on MRI 1.9.3 d374f5e
@sferik sferik Maybe this isn't necessary? c23d0b8
@sferik sferik Add back one rake task at a time 2c6de5e
@sferik sferik Go back to only testing on 1.9.2 for now 5a82670
@sferik sferik Update validates_formatting_of dependency to version 0.3.2 717aba8
@sferik sferik Fix tests 73524a4
@sferik sferik Revert "Fix tests"
This reverts commit 44e9538.
f39d4eb
@qrush qrush bump to 1.8.12 09a9d1d
@sferik sferik Add Travis-style dependency status image [ci skip] 5729eda
@sferik sferik Update gherkin dependency to version 2.6.9 fe7f09e
@sferik sferik Make status images more consistent [ci skip] f6817db
@sferik sferik Update cucumber-rails dependency to version 1.2.1 aa50892
@sferik sferik Update excon dependency to version 0.7.10 b28a77b
@sferik sferik More Ruby version pruning bfe4a8a
@sferik sferik Update excon dependency to version 0.7.12 3c83f4c
@qrush qrush don't drop downloads into a hash, just group by 2 and roll with it db5a86f
@laserlemon laserlemon Add failing Cucumber scenarios describing desired yank/unyank webhook…
… functionality
85d617a
@laserlemon laserlemon Pass failing yank/unyank webhook scenarios 015f65b
@laserlemon laserlemon Pass failing webhook specs after yank/unyank webhook integration 3c114b8
@laserlemon laserlemon Eliminate an unnecessary query from the webhook firing process 35b656e
@laserlemon laserlemon Rename the new webhook "payload" key to "rubygem." 99d9bb0
@laserlemon laserlemon Only include the legacy webhook payload keys for the original "push" …
…event
4375195
Contributor

laserlemon commented Dec 7, 2011

Rebased against master. I'm getting a few intermittent lexing errors from Gherkin but they seem unrelated since I'm getting them in master as well.

Sorry if this is unwelcome, it's not about the actual feature, but instead about the use case for the feature you mention.

If Issue #123 were complete, so the reverse dependencies of a gem were known.... rubygems could even automatically identify which gems are effected (at least as a first-level dependency) by a yank, even going so far as to see (using bundler?) if those reverse dependencies are now left in a not-possible-to-resolve-dependencies state as a result of the yank... and rubygems usually knows the contact email address of those reverse dependencies too....

you see where that's going. It could actually automatically email authors of gems left in such states by yanks of their dependencies. Perhaps as an opt-in service to a rubygems.org account, to avoid being spam.

Contributor

laserlemon commented Dec 9, 2011

@jrochkind That's right in line with my intention for the yank/unyank webhook. I already built an opt-in service like you mention (Gemnasium) and I'd like for it to be able to notify users of potentially unresolvable bundler states.

gravis commented Jun 19, 2012

Hi there, what's the status of this, please? I don't see any reason not to merge it, as yanking is a "common" operation on rubygems, it should be available in the api.
Thanks!

Hi, any hint about possible merging date on this ? or is there anything that blocks this PR ?

thanks

maletor commented Jul 19, 2012

+1

Contributor

adkron commented Nov 28, 2012

What is holding this up right now?

gravis commented May 30, 2013

Any news on this?
Thanks :)

Any new info about this ?

+1 for this!

@qrush qrush removed this from the 201307 milestone Nov 28, 2014

@laserlemon laserlemon removed this from the 201307 milestone Nov 28, 2014

Owner

qrush commented Nov 28, 2014

Holy moly this is out of date. Can we get this rebased so we can get this in finally?

Also @indirect doesn't Bundler use something like this unofficially?

Contributor

davefp commented Feb 4, 2015

@laserlemon As a result of #610 a large chunk of the repo's history was re-written. Please rebase your PR so that it will merge cleanly. Thanks!

(There are further details on the blog: http://blog.rubygems.org/2015/02/01/rewriting-history.html)

Contributor

laserlemon commented Feb 4, 2015

@davefp 😆 I can try! This pull request is over three years old 😄, so I will but only if this is still being considered for inclusion. Thanks!

Contributor

laserlemon commented Feb 4, 2015

Welp, I tried. I give up. Please feel free to reopen the issue if anybody else would like to take a shot.

@laserlemon laserlemon closed this Feb 4, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment