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

replace [twitter] badge with static fallback #8842

Merged
merged 5 commits into from Jan 25, 2023

Conversation

chris48s
Copy link
Member

closes #8837

Maybe there will be another way we could re-implement this in future, but lets do this at least as a stop-gap.

@chris48s chris48s added the service-badge Accepted and actionable changes, features, and bugs label Jan 23, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 23, 2023

Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 54489e8

@calebcartwright
Copy link
Member

I'd like to have a quick chat about this one before we proceed, will follow up in chat

colindean added a commit to colindean/colindean that referenced this pull request Jan 24, 2023
Twitter shut down the API that shields.io uses for dynamic follower count.

See also:

* badges/shields#8837
* badges/shields#8842
@calebcartwright
Copy link
Member

Actually will post here inline for transparency.

I'll preface up front that I don't have a strong opinion one way or the other, mostly playing devils advocate to expand the rationalizations against other options

  • (a) If we're going to give twitter "special treatment" by doing something different than we've ever done, then imo we either need to establish and articulate a policy we can follow going forward, e.g. "we do normal deprecation except for top X badges", or at least explicitly stake our rights as the maintainer team to make this decision on an ad-hoc basis as we see fit. I really want to avoid having any ambiguities or logically-tough-to-defned positions for any hypothetical future cases where we have to answer the "you did it for twitter, so why not for service Foo too" types of questions
  • (b) Could we achieve the same by using a more typical redirector pattern to a corresponding static badge endpoint, if yes, let's also articulate why we're taking this approach instead (e.g. high traffic volume, desire to avoid extra network round tripping, etc.)
  • (c) You made a good point in Twitter badges failing Twitter API closed down? #8837 (comment) that this (or events similar to this) have happened before. Once we make this change, I suspect we're likely to soon forget about it entirely, and it would likely be some time (if ever) before we became aware of the cdn endpoint being back online. Is that something we're concerned with, and if so, should we devise some type of tracking mechanism? I poked around through the twitter js embed code and noted that it's still internally using the same cdn references it had before, so there's a small bit of cautious optimism bouncing around in my head
  • (d) While we've all noted our intense skepticism about a positive outcome, we did have some discussion on Twitter badges failing Twitter API closed down? #8837 about trying to work with the vendor for a more feasible rate limit. I know we can work on that in parallel, but if we were, by some miracle, to be successful in that endeavor and subsequently reverted this change/updated the code to use the API then the badge would obviously change for existing users (likely back to its original state). Is that type of change something we'd be comfortable forcing on existing users?

Finally, I recognize this Twitter API issue has had widespread impact given the popularity of the badge and while it's not my intent nor desire to hit the brakes on mitigation attempts, I do think it's worth noting that there are existing workarounds available for any users who prioritize getting a different/non-error badge, really any twitter badge, over anything else. Meanwhile, I think we're far more constrained in our ability to go back and forth on changes we force down on our users

@chris48s
Copy link
Member Author

chris48s commented Jan 24, 2023

(a) If we're going to give twitter "special treatment" by doing something different than we've ever done, then imo we either need to establish and articulate a policy we can follow going forward, e.g. "we do normal deprecation except for top X badges"

I think what makes this case different in my opinion is that when we follow the usual deprecation process in https://github.com/badges/shields/blob/master/doc/deprecating-badges.md it is usually because the upstream service has completely shut down (in fact, the existing wording in deprecating-badges.md covers that).

i.e: if you look through the last 5 deprecation PRs

the service has completely gone away so there's not really any reasonable expectation that our badges are going to work or much of a useful fallback we can provide. This badge is different because Twitter is still a thing that exists. We just can't use this API endpoint any more. If twitter had completely shut down, then I think we'd follow the usual process. Also I think in other situations where something like this happens again (service is still live but we can't provide a badge any more for..reasons) we should also consider doing something similar if possible.


(b) Could we achieve the same by using a more typical redirector pattern to a corresponding static badge endpoint, if yes, let's also articulate why we're taking this approach instead

The most important reason why I don't think we should replace it with a standard redirect at this stage is because when we issue a redirect we issue a 301 Moved Permanently. 301s are cached indefinitely by browsers, so if we replace this route with a redirector and then some time later down the line we restore the previous functionality (I'm not suuuuper optimistic this will happen, but it is a possibility) a fairly large chunk of the userbase will have the redirect cached and we have no way to invalidate it. We could write a custom class that issues a 302 instead, but I haven't bothered.

Beyond that, there are a couple of less important considerations:

The fallback I've done matches the style of the label for the existing badge and renders with no message. I'm not sure it is possible to construct a static badge which gives you a message-less output. i.e: I can do

https://img.shields.io/badge/follow-@shields_io-blue?logo=twitter&style=social&link=https%3A//twitter.com/intent/follow%3Fscreen_name%3Dshields_io

or

https://img.shields.io/badge/follow%20@shields_io-blue?logo=twitter&style=social&link=https%3A//twitter.com/intent/follow%3Fscreen_name%3Dshields_io

but I'm not sure we can construct a static badge URL that outputs

fallback

I might be wrong? I don't think that's a deal-breaker though.

Also we don't have a way to define an examples[] on a redirector (this could be a good thing or a bad thing). Arguably we should remove the example? Open to thoughts on it.

The most crucial point is I don't think we're ready to issue the 301 yet though.


(c) You made a good point in #8837 (comment) that this (or events similar to this) have happened before. Once we make this change, I suspect we're likely to soon forget about it entirely, and it would likely be some time (if ever) before we became aware of the cdn endpoint being back online. Is that something we're concerned with, and if so, should we devise some type of tracking mechanism? I poked around through the twitter js embed code and noted that it's still internally using the same cdn references it had before, so there's a small bit of cautious optimism bouncing around in my head

I suspect if this endpoint was going to start working again it would have done it by now. Personally I don't plan to invest a lot of energy in monitoring or seeking alternative solutions. I suspect this service is popular enough that if it becomes possible again and anyone cares, someone will tell us in an issue.


(d)... if we were, by some miracle, to be successful in that endeavor and subsequently reverted this change/updated the code to use the API then the badge would obviously change for existing users (likely back to its original state). Is that type of change something we'd be comfortable forcing on existing users?

Yes. I'd be happy to assume that most people would consider restoring the original functionality a fix.

@chris48s
Copy link
Member Author

The other thing that occurs to me here is both these badges have no dynamic content, so we can cache them for longer.

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sufficiently persuaded and agree with your analysis. Appreciate you taking the time to articulate things, I know I'll feel better about having this for posterity and being able to point back to it.

I'd like to continue part of the dialog around deprecation conditions, but that can certainly be done in parallel and doesn't need to block

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Twitter badges failing Twitter API closed down?
2 participants