Summary

Add UUID to the "Add Repository" form and Repository API. Modify post-review to take advantage of this new information.

Review Request #749 — Created Feb. 20, 2009 and discarded

Information

Owner

byoteki

Repository

Review Board SVN (deprecated)

Branch

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

With a large amount of repositories post-review would take a long time because it would iterate over each one doing an svn info against them to get the UUID. This change gives you the option to include the UUID of the repository in the "Add Repository" form allowing you to limit post-review to a single GET API call. If you do not enter a UUID it will fail through to the old behavior. This change is very small but it inherently modifies the schema, adding a UUID column to the scmtools_repository. I admittedly do not know enough about Django to know how this will effect users upgrading.

Testing Done

I tested locally on my machine with a fresh install of reviewboard.

I'm trying to decide what I think about this change. Maybe I'm not clear on how it should be used. The UUIDs, I assume, are user-defined, right?

We're already doing the UUID check, so we're actually just introducing more overhead by doing a second loop through the repositories, and the end result shouldn't really be any faster.

If we do decide we want to add this UUID field to the schema, we'll need a migration definition (see reviews/evolutions/*.py for examples).

/trunk/reviewboard/contrib/tools/post-review (Diff revision 1)

We're still looping over every repository to find this UUID, just like we are below. I don't see how this is more efficient here.

byoteki Feb. 23, 2009, 11:24 a.m.

The difference is we're saving the work of the external call per repository. With this change it allows a user to add the UUID as a user defined field and when post-review runs it only has to iterate over a list and do a key lookup opposed the the overhead of connecting to each repository through the defined protocol, running an svn info, and returning. It's one API call instead of n. The way I've coded it though is so if you don't find the UUID in the data structure provided by the API it will fall through to older, less efficient method. Let me know if you need further clarification.

chipx86 March 4, 2009, 5:29 p.m.

Sorry for the delay on this.

Rather than a UUID, perhaps we should just allow the existing ID to be specified for a repository. Since repository IDs can't change without causing major problems anyway, they're probably Good Enough.

byoteki March 4, 2009, 11:24 p.m.

I'm not sure I understand how that would work. When you're in your local checkout and you run post-review it's not aware of the ID that review-board has assigned to a repository and since some RCS's have different protocols for checkout we can't just compare on that for a match. Using the UUID that is assigned the repository makes the most sense here in my opinion. I could just be misunderstanding what you mean, I'm open to any solution that decreases the amount of info calls against the repositories and increases speed/efficiency.

david March 11, 2009, 7:27 p.m.

I think Christian is just misunderstanding. This approach seems fine to me.

chipx86 March 11, 2009, 7:33 p.m.

I was. I was thinking in completely different terms.

Okay, this seems fine in concept, but you'll need to add a database evolution for this still, in order to add that field to new installs. See the reviews/evolutions/* files for an example of this.

Also, it might be nice to have the UUID field filled in automatically by SVNTool.get_repository_info in scmtools/svn.py if it's blank. That way the admin doesn't even have to enter this. After the first repository scan, it'll be done automatically, speeding things up.