Comments

There are some issues that have events missing within the maintner corpus. This makes it impossible to create an accurate milestone burndown chart where you want to query for the state of an issue at a particular time window.

A few examples of issues in maintner that have incomplete event lists:

This comment has been minimized.

Would you be so kind as to point me into the right direction here? I think I know where to start digging.

First off, I want to write a test to reproduce this bug. Ideally a unit test something like:

funcTestParseMultipleGithubEvents(t *testing.T) {
}

I think the right code part here is

funcparseGithubEvents(rio.Reader) ([]*GitHubIssueEvent, error) {

... if I see that correctly. Somewhere here is the bug buried. Am I poking in the right direction? The manual way of seeing the missing event? I fired up the maintserver and located said issue, but can't find the missing event as the comment is there and the issue is closed. I'm guessing it's somewhere else?

This comment has been minimized.

@Skarlso can you provide more context as to what your hunch is? I can’t surmise what you’re trying to figure out. Whether we’re being rate limited? If we were, then a lot of other requests would also fail subsequently and we’d see a lot more issues.

And this is the main function that gets the events in the first place. And there are a lot of error possibilities here and this is largely untested code. So my hunch was that there is an error at some point in the function not necessarily because this function might be wrong. But.... I'm writing the unit test for it which either way sheds some light on the matter. Hopefully.

EDIT: The other hunch is my first hunch that I'm following. That parseGithubEvents is incorrectly parsing something. I begun writing a test for that. :D It's just tedious. :)

This comment has been minimized.

@andybons Alright. So... In the attached CL I added a bunch of tests around every sync event that happens that parses an incoming github event directly parsed from the API.

If you are aware of any part of the code any where else this could still fail, please tell me.

These tests all pass. What I also added is a retry logic around rate limiting.

I think right now the CL is solid as in adds a bit of logging and a lot of test cases for the area of the code that was not tested before. As is I think it's ready to merge, even though it does not fix the problem yet. ( unless the rate limiting does kick in from time to time, even though you are saying then other stuff should fail. ).

I have not stopped investigating, I just merely proved that THIS part of the code DOES work and correctly parses event logs.

This comment has been minimized.

@andybons Hi. I opened a separate issue with the test coverage increase and the github throttle handling.

I searched for a long time, I believe it's either a race condition in sync logic, a timing issue, or github throwing some kind of error somewhere which is not being handled.

I created an issue which fixes the throttling which might fix this issue, but I ran out of ideas. Without logs that I could investigate on what's happening this is an insanely hard task with my code base knowledge.

This comment has been minimized.

edited

Without logs that I could investigate on what's happening this is an insanely hard task with my code base knowledge.

Sorry, I couldn't help out.

Understandable, and no problem.

On the contrary, it is very helpful that you've investigated this far and shared your findings. This information will make it easier to make further progress towards identifying the root cause. Thank you very much @Skarlso, your efforts are greatly appreciated!