Page MenuHomePhabricator

Better handle duplicated audits/maniphest tasks
Closed, ResolvedPublic

Description

This is a bit of a grabbag task, since I don't know exactly what's going on. The basic problem is that someone will fork a repo, and for some reason the commit hash is different, or maybe the commit hash is the same but the repo is different, but in any case phabricator sees all the commits in this new repo as new, and thus re-sends mail for all tasks and audits in the repo. (Presumably it would send out mail for other things too if we had herald rules set up for them.)

We end up with tasks like http://phabricator.khanacademy.org/T1619, with:

Commits D4237 / rMWB7b0f02a72e08: Handle natural language text inside tag attributes, eg <input value="text">
        D4237 / rMWB5031b6808140: Handle natural language text inside tag attributes, eg <input value="text">
        D4237 / rGW377cfa96bc79: Handle natural language text inside tag attributes, eg <input value="text">
        D4237 / rGWa884a0184530: Handle natural language text inside tag attributes, eg <input value="text">

I don't really understand how this repository got 'forked' -- it may be a kiln thing (we use kilnhg.com as a backend for this repo). It's probably user error that it got forked in a way that caused all these tasks/audits to be created. But it would be helpful if we could deal with it better.

One possibility is to just make cleanup easier:

Deleting the audits is pretty simple, using bin/audit. However, having people delete the mail is difficult. While all the problematic mails mention the repository, they do so as part of a string: csilvers added a commit: rMWB5031b6808140 or the like. I'm unable to search for the repo (MWB) in gmail, because gmail only searches on entire words.

A simple solution to this would be to include the repo-callsign somewhere in all herald emails. Sadly, gmail doesn't let you search on arbitrary mail headers, which would be an obvious place to put it. But maybe there's a reasonable place to stick it in otherwise? For maniphest tasks and diffusion audit mails would be enough for us.

Another possibility would be to avoid creating the audits in the first place (and perhaps the task updates too?) I don't know if there's a reasonable way to tell that an audit is a 'dup' when the commit-ids don't match.

If you want, I can try to get more information about how these 'forked' repos are created. I don't know if that will help guide the decision here or not. Probably just having an easy way to mail-filter would be good enough.

Event Timeline

csilvers raised the priority of this task from to Needs Triage.
csilvers updated the task description. (Show Details)
csilvers added a project: Phabricator.
csilvers added a subscriber: csilvers.

This might be fixed by D7429, or followup work on T3217. These add an "importing" flag to repositories, which basically works like:

  • When you create a repository, the "importing" flag is set.
  • When the parsers catch up to HEAD on a repository (i.e., have processed all preexisting commits), the "importing" flag is cleared.
  • While the "importing" flag is set on a repository, no revisions are closed, no tasks are linked, no emails are sent, no audits are triggered, no feed stories publish, and no Herald rules act in response to commits being discovered there.

Does it sound like that would cover it?

The more general version of this issue is:

  • User alincoln has Herald rule "On commit; always; send me an email".
  • A perfectly normal preexisting repository is imported (no Klin or duplicate hyjinx required).
  • User alincoln gets 30,000 emails.

Ah, I think that would solve it indeed!

epriestley claimed this task.

D7429 is in HEAD now, so I think this should work in the future. Let us know if you run into it again, or I missed some special cases, etc.

(Repositories in this "Importing" state will show "Importing..." in the header in Diffusion, and there will be more visual cues and status information landing soon.)