Receiving issue that is already closed

[sngeth]

Is the intended behavior to receive issues that are closed or was this a 1 off error?

Hello @sngeth,

Here are some issues you should check out:

#17104 Fix numericality validation with large numbers

rails/rails about 1 year ago

[sngeth]

Sorry, I just noticed this was a Pull Request item not an Issue

[prathamesh-sonpatki]

@sngeth Still we can change our code to not send such PR which is already closed, want to send a PR? 😉

[schneems]

There is a race condition in sending out emails. We only import from GitHub once a day and the issue may have been closed between when we import and when we send out the email. Maybe the best way to check would be before we send out an email for an issue, we see if it's been updated within the last say 30 minutes. Otherwise we would be adding N API calls to github where N is the number of users times the number of issues they've subscribed to.

[sngeth]

Hey I am interested in adding this improvement although i'm not sure if i'm clear on your suggested strategy.

Are you saying we should repopulate our entire Issues model before SendDailyTriageEmailJob#perform gets fired thus keeping things O(n) where n = issues.

Is there a specific way to determine which are the "pertintent issues" that needs to be taken account without tying it to # of users or subscriptions....or just do it wholesale?

I also noticed that User#send_daily_triage! calls IssueAssigner#assign which calls Issue#valid_for_user? which calls Issue#update_issue! which refetches the issue. Unless i'm reading something wrong, does this not solve the issue of stale statuses?

def valid_for_user?(user, skip_update = Rails.env.test?)
  unless skip_update
    update_issue!
    ...
  end
  ...
end 

It looks like it's already doing the original N * Users * Issues ?

[nathany]

I'm seeing closed issues as well. This is the most recent one: https://github.com/golang/gddo/issues/233

That issue was closed on Sep 25, 2015 yet I received it in an email today.

[schneems]

There may be something broken in our close issue task

[sngeth]

@schneems how often does the close task run?

Only thing I could see why @nathany would be receiving the issue was that the update_at returned 11/25/2016 despite being closed over a year ago

[lostapathy]

There is apparently still an issue here. I just got an email from codetriage (my first in a LONG time) that suggested I look at #609, which was closed 18 days ago. (oddly enough due to a PR of mine that superseded it)

[schneems]

Ugh, i'm guessing something is broken in one of the tasks :(

Have you gotten more that are closed since?

[lostapathy]

I haven't noticed any, but I only really noticed the last one because I had worked it. I'll take a look at the rake tasks at some point and see if I can find anything, but I may need you to run something against production data to validate.

[vnen]

I don't always receive emails and when I do they are already closed. I wonder if the volume of issues in https://github.com/godotengine/godot is giving some trouble to the system.

[schneems]

We only update issues once every 24 hours, and then once again right before we send it out here https://github.com/codetriage/codetriage/blob/b92e347e0f4714b4646be930e341be5a44761b95/app/models/issue.rb#L8-L16.

Are they recently closed or closed a long time ago? Can you point me to a few of the issues that you've gotten?

[vnen]

They were closed long ago. These are the two I got yesterday:

• https://github.com/godotengine/godot/issues/9335
• https://github.com/godotengine/godot/issues/9744

[schneems]

Weird. I'm seeing that they shouldn't have been sent:

irb(main):013:0> user = User.where(github: "vnen").first
irb(main):014:0> Repo.where(full_name: "godotengine/godot").first.issues.where(number: "9335").first.valid_for_user?(user)
=> false

[klyonrad]

I also received a notification for an issue that was closed 14 days ago. So another point against the 24 hours caching issue :/