-
-
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
[YOUTRACK] Add YouTrack Issues #10011
base: master
Are you sure you want to change the base?
Conversation
|
So I checked this out locally. Before I give a round of comments on the diff, I tried requesting Do you get the same? In order for this to be a viable badge, I think we can work round a bit of flakyness, but it is going to mostly need to return a valid badge. I don't want to spend too much time bikeshedding the code if this just isn't going to work reliably. |
It seems the YouTrack API is a little flaky there. The word 'bug' doesn't exist in any of the cards so instead of returning 0 it returns -1 constantly. If I use
Searching for I'll update the example as that was bad and an oversight. One thing I have noticed is that in their own UI tags like |
It is a bit annoying that One other thing I have noticed is that once the query has been evaluated, it only seems to stay cached upstream for quite a short amount of time. Maybe a minute or so. Then you're back to "processing". I know there is some sunk cost here because you've already put some time into implementing this. Do you think what's here right now is reliable enough you'd put it in your README? It feels to me like this is a bit too flaky to be useful/supportable at the moment. Another thing I have noticed is: if you get back a -1, you seem to be able to make a second request and get back a valid result pretty quickly. Do you think that is true more widely, or does this only happen on the projects I'm looking at because they have a really small search space? |
If I did not sink time into this, we would not know about how flaky their API is. It's been a good learning curve of some areas of Shields new to me too. YouTrack doesn't have a real DB for its backend which may be why they do not cache very well. In the current state of their API responses, I would agree that this is not good enough for a README.md. In general they advice retrying after a few seconds with a -1, which I believe is okay for an application that would use their API. But badges needs to be quick. I did consider one last idea which was the following logic:
Given that -1 can be returned instead of 0 due to just a bad query (syntax wise valid, but data wise invalid), I was hesitate to implement this logic to avoid an infinite loop scenario. With the x retries attempts, I am concerned about the negative performance it would have on shields.io. If a retry mechanic is ok, I can give that a shot. If not, we can close this out due to upstream not being reliable enough at this time. |
Before we go down that route, do you have an example of a project/query which involves searching a large number of issues to evaluate the query? I'd like to get an idea of how long/short an amount of time we would need to wait before retrying in that case before we put more time into coding solutions. |
After testing and discussion on discord this PR will go into Draft Mode until such a time that YouTrack's API is more reliable. In the current state an API can be processing anywhere from 1 to 20 seconds, this is not an acceptable response time from the upstream API. I will leave my branch up and periodically update it. Hopefully JetBrains improve their API in the future. |
Add support for YouTrack Cards/Issues as per #9997
A new Service Token and test is added, InvalidResponse of processing added when a -1 is returned, and cached for 10 seconds. This is expected when YouTrack isn't finished and is documented at https://www.jetbrains.com/help/youtrack/devportal/resource-api-issuesGetter-count.html#create-IssueCountResponse-method
For CI purposes shields.youtrack.cloud was created and is used throughout all tests as this instance is under our control. The guest user is enabled read-only so that the CI tests can work in Github Actions without needing a token, but prevents people breaking the tests.
Only concern is whether or not cachedSeconds is working as IntelliJ IDE reports this doesn't match InvalidResponses definitions, but seems to work. And also, during testing when the cloud instance is idle for long periods, a timeout is received on the first test of a day, as such I added 'timeout' as a possible test response.