Page MenuHomePhabricator

Add an "importing" state to repositories and clean up the UI
ClosedPublic

Authored by epriestley on Oct 27 2013, 3:17 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 12, 1:24 AM
Unknown Object (File)
Mon, Dec 9, 5:42 PM
Unknown Object (File)
Sun, Dec 8, 3:14 PM
Unknown Object (File)
Wed, Dec 4, 1:30 PM
Unknown Object (File)
Tue, Nov 26, 3:07 PM
Unknown Object (File)
Tue, Nov 26, 3:07 PM
Unknown Object (File)
Tue, Nov 26, 3:07 PM
Unknown Object (File)
Tue, Nov 26, 3:07 PM
Subscribers
Tokens
"Mountain of Wealth" token, awarded by btrahan.

Details

Summary

Fixes T3217. Ref T776. Ref T1493. Broadly, this introduces a mechanism which works like this:

  • When a repository is created, we set an "importing" flag.
  • After discovery completes, we check if a repository has no importing commits. Basically, this is the first time we catch up to HEAD.
  • If we're caught up, clear the "importing" flag.

This flag lets us fix some issues:

  • T3217. Currently, when you import a new repository and users have rules like "Email me on every commit ever" or "trigger an audit on every commit", we take a bunch of publish actions. Instead, implicitly disable publishing during import.
  • An imported but un-pulled repository currently has an incomprehensible error on /diffusion/X/. Fix that.
  • Show more cues in the UI about importing.
  • Made some exceptions more specific.
Test Plan

This is the new screen for a completely new repo, replacing a giant exception:

{F75443}

  • Created a repository, saw it "importing".
  • Pulled and discovered it.
  • Processed its commits.
  • Ran discovery again, saw import flag clear.
  • Also this repository was empty, which hit some of the other code.

This is the new "parsed empty repository" UI, which isn't good, but is less broken:

{F75446}

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Just an FYI, but this should fix (or at least improve) some of the errors you hit earlier. It still isn't practical to make it very far through discovery on hosted repositories, though.

I'm going to give the state flag more values so we can hold clearing the flag until all the Herald stuff has happened, and also maybe draw progress bars.

I figure you intend to give the state flag more values in a future diff? If not, go on ahead and update this bad boy and I'll take another gander.

I think this diff will just get one change, along the lines of importStatus = 0 -> importStatus != SOME_FULLY_IMPORTED_CONSTANT -- I'll yell if it's more extensive than that.

epriestley updated this revision to Unknown Object (????).Oct 29 2013, 10:06 PM
  • Check for commits with importStatus not equal to IMPORTED_ALL.
  • Also don't autoclose based on anything discovered in "importing" repositories.