-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[openuserjs] Add OpenUserJS service badges #8081
Conversation
|
Closes OpenUserJS#1961 Post OpenUserJS#1755 , badges/shields#5505 , and may eventually apply to badges/shields#8081 NOTE: Rating is inclusive of votes/flagging/moderation so no real need at this time to propagate those.
Thanks to all involved, it's nice coming into a PR that's already had thorough discussion and review 😄 Couple questions before I dive in:
|
The existence of the page is documented at https://openuserjs.org/about/Frequently-Asked-Questions#q-does-openuserjs-org-have-meta- The
@Martii may have a full answer. From what I've seen, usual requests to |
Thanks @DenverCoder1. Would be good to get a feel for what those are and to see what our options are. We do have some caching knobs we can tune but want to know what we're working with. Also as an aside, you don't necessarily have to worry about updating the branch until we get close to being ready to merge. It's more of a final check we have in place to make sure there's no conflicting work and that the changes will be fine atop the trunk branch, but it's not necessary to add merge commits any time an unrelated PR gets merged. |
Currently no... see: This could change if it is abused but so far knock on things. We're thinking about pruning
That's because it's a HTML response not a JSON response so it has a different rate limit set. Should probably be in this module service as a check perhaps. This is where I get fuzzy on the Shields side. |
I appreciate you sharing relevant snippets from the codebase but I'm going to ask some follow up questions anyway to avoid incorrect assumptions about the source code, gain a better understanding of potential future plans, to account for anything else in the system architecture that could be at apply above/before the running app, etc. the meta json endpoint is the only one we'll need
|
Correct and what I've already stated.
Any time the script gets updated which can be as little as never or as maximum as reposting allows (hence the short period you mentioned next). We do however cache in a browser for a maximum of 7 days for HTTP 1 caching and immediately on eTag for HTTP 1.1 if changed.
Same as your first question bullet point answer here.
If you have an endpoint similar to GitHub's (for synchronizing a repo here to us... mentioned at https://api.github.com/meta ... possibly much simpler than GH's of course) we'd be more than happy to add Shield's server IP to a special list. Also note other UserScript sites (including GitHub) has rate limiting so most of these questions probably should be asked/checked as well. 😸 |
I appreciate you taking the time to participate in the review and dialog here as we consider potentially adding a feature to our platform that would primarily be utilized by users of your platform. However, please keep in mind that we integrate with and provide badges for hundreds of platforms/services, the overwhelming majority of which, including OpenUserJS, we do not use ourselves and have no familiarity with. You are obviously very familiar with your service, but the discussion here has been heavy on acronyms and tribal knowledge, and relative to the other services we integrate with, there's not a lot of API documentation through which we could edify ourselves. We're still trying to play catch up, and clarifying questions are being asked because we don't feel we had sufficient clarity. Even if things looked and felt sufficiently clear to you, I'd still kindly ask that you refrain from the "already answered" types of comments and their associated implications.
Understand the inclination but this won't be viable for us. Our runtime is dynamically scaled and ephemeral with dynamic IP addresses. As such when we engage with vendors/maintainers of platforms that have concerns around load, we typically look to adjust cache settings on our end and/or change to a model where the Shields requests are authenticated using an account the vendor/maintainers have granted a higher rate limit. However, this doesn't sound like something that can/should be solved in the abstract, especially given your confirmation around the not found rate limiting behavior. If the traffic Shields sends to OpenUserJS starts to become a problem down the road, please don't hesitate to reach out and we'll work to find an appropriate solution (we send info in the request headers that could be used to filter our traffic from the rest).
Sorry, but I don't understand what you're saying at all. Are saying that we need to ask/investigate the questions I've asked you about OpenUserJS against all the other services we integrate with? If so, thanks, but we're all set. I ask these questions of you because these are the standard set of questions we consider when adding support for a new service/platform, and we're very, very familiar with GitHub's rate limits and how to work with them at mass scale. |
I am quite familiar with that and we've been using Shields for about 10 years and I've personally dealt with this repo before. 😄
Apologies that your perception was skewed on this. Directness is our goal not beating around the bush.
So is GitHub but we'll just treat Shields in the same traffic pattern as a Guest then. No big deal. Thought I'd offer an improvement on Shields via our site, if it wasn't already available, since the retrieval on NPM is about 98% instead of the alluded 100%... but is mostly reliable compared to others.
That's actually why we are here now to keep the bugs to a minimum for now. 😸 And of course... there's always the dynamic badges anyhow for Authors to utilize if things change outside of my current Active Maintainer scope.
Recommending being impartial yes as to not be monopolistic in nature.
There's more than just GitHub. We chose GitHub as a comment discussion point as it is familiar to all of use since we are all here for objectivity. You skipped this validation within the last three weeks. Just hoping for some objectivity from Shields instead of what our team has observed and perceived as a dual standard. Again we don't beat around the bush and we do appreciate your service as do others. Anyhow.... if you feel the need to ask more questions I am available at your leisure but you are reminded that literal answers will be given at all times through my successful communications and lifetime accolades. @DenverCoder1 has plenty of accolades as well and this is the first we have enjoyed dealing with his organization/himself as he keeps his objectivity which is what our community strives for. 👍 |
@Martii - I was trying to give you a gentle nudge but allow me to be more direct this time. You may believe you're simply being blunt/literal, but your comments are coming across with an increasingly flippant tone. I need you to stop and ask you to be mindful of this caution should you choose to continue engaging in the conversation. Your comments give the impression of at least being mildly irked that I posted a summarizing/clarifying comment, and then noted that impression. Perhaps I misunderstood, but that's how they come across, and when it comes to intra-human communications I think the phrase "perception is reality" holds a lot of water. I hope it was just a simple misunderstanding, but I also apologize if I've offended in any way. Again, I understand you feel you've been unequivocally clear in your brief commentary, but please remember that your perception doesn't inherently mean your shorthand is actually clear to everyone else. It wasn't, but we're in a better state now.
This is over the top, and unnecessary. This sounds as if you feel OpenUserJS is being unfairly scrutinized, which is absolutely not the case. I will make the assumption that you are referring to the recent GreasyFork badges and are trying to conflate that instance to paint a broader, and incorrect, set of characterizations. To the best of my knowledge, neither of these two services had public documentation around their respective rate limits, however, rate limits still became a discussion point here because they were directly observed by the contributor and you were present as a maintainer of that service. There's a pretty standard set of criteria we consider when adding badges for a new service, and when applicable, that includes rate limiting on the upstream service too. Not every one of those considerations is a "blocker" per se, but are things we try to wrap our heads around for each service. I'm simply trying to ensure we have due diligence on our part to make sure we're operating as a good citizen with your platform and that we can provide a good badge experience to our users, nothing more and nothing less. Also want to stress there's no hard feelings, and I hope we'll be able to get these badges incorporated. I just wanted to make sure we keep the conversation constructive, respectful, and in accordance with the the type of dialog we strive for here in our community. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Minimizing the last comment as it's rather adversarial and there's no need to continue that thread which is increasingly off topic. Thanks for your feedback on the submitted code, technical insight, and willingness to answer questions about your service. We'll be sure to reach out if there are any additional questions you can assist with! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @DenverCoder1! This is in a pretty good state.
I left a few inline items below on mostly small things, and noted another one I want to check on too.
|
||
export default class OpenUserJSDownloads extends BaseOpenUserJSService { | ||
static category = 'downloads' | ||
static route = { base: 'openuserjs', pattern: 'dt/:username/:scriptname' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which noun do you think will feel more natural to OpenUserJS users, installs
or downloads
? I gather based on the label value and what was done/discussed for GreasyFork that it's installs
, but that has me rethinking the route a bit because for other badges that opt for installs
over downloads
that's typically reflected in the route, e.g. Jenkins Plugin Installs - https://img.shields.io/jenkins/plugin/i/view-job-filters
Our docs on badge url structure do mention using the d*
pattern for downloads or installs, but given the presence of i
elsewhere I'll double check with the other maintainers to confirm
And somewhat of an aside, but if we do decide to shift to i*
for installs then we can consider updating the corresponding GreasyFork route too (we've got mechanisms to update routes while maintaining backwards compatibility, will share if and when we need to cross that bridge)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
installs
is the terminology used by OpenUserJS. This includes usage on the website, the meta JSON, and confirmed by the OUJS admin Martii in comments elsewhere.
I suppose I'll leave it as d*
for now until that's decided?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, leave the route as-is for now and I'll follow up with a resolution one way or another. I believe using the download nomenclature in the route is intentional but it's been a while since we last discussed and my memory is a bit fuzzy on where we landed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll likely continue discussing the specifics of this offline, but for now I think we've reached a sufficient consensus that it's fine to go with this route as is. We can always add redirectors (our mechanism for updating routes without impacting existing users) for these badges down the road if we end up deciding to pivot on the route conventions.
I'll try to make time over the weekend to do a final pass over the subsequent updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I see after reading through the previously linked issues that the maintainers of the upstream service still have some concerns, so we'll put this on hold for now until we figure out whether there's a viable path forward or whether we should just abandon this.
It seems everything should be resolved now (besides waiting for the route name for installs) |
I was really hoping this matter was settled, but given subsequent discussion OpenUserJS/OpenUserJS.org#1961, and particularly OpenUserJS/OpenUserJS.org#1961 (comment) onward, I see that it is not. First and foremost, everyone that decides to participate in our community is inherently agreeing to our Code of Conduct, and that includes being subject to moderation from those of us that lead this community. As stated in that document, we, the leaders of this community, are responsible for making the determination about what is, and what is not appropriate. Whether or not you like or agree with the rules of our community, or the decisions we make around appropriateness, does not change the fact that we have the final, and only say on this front within our community. This all started because the way you had been communicating within our community on multiple occasions, particularly when responding to questions from both contributors and maintainers, was not really in accordance with the type of dialog we strive for here. It was still very mild at that point, and I assumed it was unintentional on your part, so I gave you what I believe was an in-kind, very mild moderation check in #8081 (comment) in the hopes you'd have a typical "sorry, wasn't my intent" type of response. In retrospect I probably should have been more explicit about that motivation for that check, so apologies if that wasn't clear. Unfortunately your reaction to that check, and especially the reaction to the subsequent moderation check, was simply inappropriate. I recognize that you disagree, that you think your behavior is fine, and that you did not appreciate those checks. However, your perception of the appropriateness of your behavior does not particularly matter in this context. You do not dictate the terms here and you do not get to determine what is and what is not appropriate in our community. This is not up for debate; there is no need for you to defend or rationalize your prior responses, nor is there any utility in critiquing me or the project, so please do not do so again. I have no intention of responding to the various name calling, misquotes, and insinuations from your previous comments above and in the linked issue in your project, however, I recognize that in some of those same comments you noted some continued technical concerns and I agree with you that we would need to discuss and come to an agreeable resolution before we could consider moving forward with this PR. I am willing to discuss those specifics here, provided you are willing and able to do so in a different manner, one that's in accordance with our community rules and goals, even if it requires agreeing to disagree about the prior discussion. (For what it's worth I'd also happy to answer questions over in your community if you'd prefer that, although it sounds like you preemptively banned me over there) However, if you have decided that you would instead prefer that Shields not integrate with OpenUserJS, for any reason, we will absolutely abide by that, and abandon this PR. We could also consider an enhancement to our DynamicBadge feature to prevent users from leveraging that to produce OpenUserJS badges. I'm unclear on what if any relationship exists between OpenUserJS and GreasyFork, but the above can apply to GreasyFork as well if that's desirable/necessary. Please let me know if you'd prefer that we abandon the OpenUserJS badges, or if you'd still like to see if there's a viable path forward, in which case I'll try to respond to some of your commentary around rate limits and dos. |
I'm coming in to this one late, but I've read over the various threads and issues and also discussed this with Caleb away from GitHub. I'm posting the final reply on this issue as Martii has said elsewhere he has blocked Caleb and I want to make sure everyone sees this response. Our assessment is that while the code here is of good quality, if we take these badges on there is a good chance of traffic from shields.io to openuserjs.org being blocked or rate limited in the near future leading to poor experience for users or disproportionate support/maintenance overhead for us a maintainers of the service, at least at this point in time. When it comes to DoS protection, we rely on CloudFlare to protect us from Denial of Service attacks and the upstream APIs we call also transitively benefit from that protection. ~9 million malicious requests were blocked or challenged by CloudFlare in the last month, and that is ~9 million malicious requests that were not forwarded to upstream APIs we integrate with. It provides adequate protection for us and the other services we integrate with. @Martii - Caleb has already said this, but also you have said elsewhere you've blocked him so I'll repeat it for clarity. As well as not adding these badges, if you'd also prefer us to block traffic to openuserjs.org via other routes (e.g: the dynamic badge), let us know and we can block it. If not, we'll continue to allow users to self-serve via the dynamic badge. @DenverCoder1 - I appreciate it is disappointing this will not be merged after you've put a lot of time into it and this is not because of a problem you have caused. You've made some other great contributions recently and I hope this won't sour your experience of contributing to the project. |
Fixes #5505 (when merged along with #8080 being merged)
Partially resolved - #829
API endpoint - https://openuserjs.org/meta/username/scriptname.meta.json
Documentation - https://openuserjs.org/about/Frequently-Asked-Questions#q-does-openuserjs-org-have-meta-