-
Notifications
You must be signed in to change notification settings - Fork 66
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
Buildasaur post multiple comments for the same integration #147
Comments
(Now some stupid questions:)
|
What I think it may be is that my integrations take as long as 20 minutes to finish. Buildasaur timer is set to 15 seconds. Maybe the Xcode server API takes some time to close the build (or perhaps some responses are cached?) and if that happen in that 15 second span, Buildasaur then post a comment. Ps: what happened to showing in the report the results from multiple simulators? |
On the list, #49. |
Looking at the source code I think what it may happen is that if github is slow and the requests take anytime greater than the sync interval (default 15 seconds), there is a good chance that the post comment requests get stacked and queued because the status won't be updated. I think the only solution would be to actually add some more check or limit the number of post comment requests per commit (PR) to 1. In that case maybe post only the most recent one and cancel the previous one? |
I've disabled all caching for this exact reason, but yes I've seen GitHub actually returning stale data for about 10 seconds after a change being made on the server. The way it works now is that the Syncer checks what the current commit status is and what it's supposed to be. If they're different (and the transition is At the moment, the only thing I can recommend is making the sync interval longer, let's say 30 seconds. To be honest, I have it set to short intervals mostly for testing + for projects where tests only take less than a minute. If you think about it, if your tests take 20minutes, delaying that by other 15 seconds won't make a difference in the grand scheme of things. So yeah, make the interval longer and let me know if it keeps happening at 30 seconds. Right now there's no easy fix. However, long term, this should be fixed by #62, which will allow us to make many more API requests without them being counted towards our rate limit (so we'll be able to check whether this comment is already posted). |
Ok, @valeriomazzeo, turns out El Cap changed the way This is fixed by #174, which once again blocks caching from happening, thus fixing this issue of repeated comments. I tested this with 5sec sync interval and it only posted once. I'll create a release with that now, so look out of 0.6.1, which will have this fixed. Thanks for reporting this! |
nice 👍 |
The text was updated successfully, but these errors were encountered: