On June 9, 2014 at 11:05:45 AM, Leander Hasty (lean...@1stplayable.com) wrote:
We just completed an upgrade to reviewboard 2.0.1 and rbtools 0.6. We 
had to patch rbtools a bit, though (we maintained similar patches on 
post-review in the past). 

Some of these are unquestionably *not* the best way to be working 
around the issues we're seeing, and I thought I'd share and ask for 
some input, then possibly try to clean up and submit a pull request, 
or a few bugs, or whatever makes the most sense. 
We’re always happy to receive patches that improve things for everyone.

I should note that we don’t accept pull requests. All patches go through 


As a bit of background: our primary usage is in an svn post-commit 
hook, run by apache. The command would look like this: 

/usr/bin/rbt post \ 
--debug \ 
--publish \ 
--submit-as=leander \ 
--server='https://rb.example.com/' \ 
--username=buildsystem \ 
--password=ELIDED \ 
--repository=examplerepo \ 
--repository-url='file:///data/svn/examplerepo' \ 
--repository-type=svn \ 
--summary='examplerepo 2349 - Testing rbt post[...]' \ 
--description='rev 2349 - Testing rbt post, ignore this commit.' \ 
--target-groups=examplerepo \ 


The first patch is to address the second request to rb API failing, 
but the first succeeding: 

>>> Making HTTP GET request to https://rb.example.com/api/ 
>>> [...] 
>>> Making HTTP GET request to https://rb.example.com/api/review-requests/ 
>>> Got HTTP error: 401: <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN"> 
<title>401 Authorization Required</title> 

In rbtools/api/request.py, in PresetHTTPAuthHandler's http_request 
method, we comment out "self.used = True", which seems to prevent 
subsequent requests from getting the basic auth header. I'm not sure 
if this naive removal is causing the basic auth header to be added 
twice in some cases. 
This does not occur normally. It should retain the cookie information used to 
stay authenticated.

Does that hook have a $HOME set? It should be some place where the information 
can be written.

What version of Python are you using there?

Do you ever see this problem outside the hook?


The second patch is to address the server's understanding of our 
repository's URL (https://svn.example.com/whatever) not matching our 
local understanding (file:///data/svn/whatever) -- if I'm reading 
things correctly. 

This -- in rbtools/clients/svn.py's SVNRepositoryInfo class, in 
find_server_repository_info -- is just a pessimistic fallback to "it's 
not relative, assume it's at root", see the "# 1P ADDED" comments: 


# We didn't find our locally matched repository, so scan based on UUID. 
for repository in repositories: 
info = self._get_repository_info(server, repository) 

if not info or self.uuid != info['uuid']: 

repos_base_path = info['url'][len(info['root_url']):] 
relpath = self._get_relative_path(self.base_path, repos_base_path) 

if relpath: 
return SVNRepositoryInfo(info['url'], relpath, self.uuid) 
else: # 1P ADDED 2014-06-08 # 
return SVNRepositoryInfo(info['url'], '/', self.uuid) # 1P 
ADDED 2014-06-08 # 

# We didn't find a matching repository on the server. We'll just return 
# self and hope for the best. In reality, we'll likely fail, but we 
# did all we could really do. 
return self 


I'm not even sure this one is 100% necessary, I need to look a little 
more. But it seems assuming root is perhaps better than just 
returning self in this case? I need to step through this code a bit 
more thoroughly to understand what is going on here. 

We could use "https://svn.example.com/whatever"; as our 
--repository-url arg, but this requires that the "svn info" commands 
have authentication cached somewhere, which turns out to be a little 
tricky in our usecase (running as apache user with invalid homedir). 

We strongly recommend using repository names and not URLs wherever possible. 
This provides the fastest lookup, and skips the whole issue of differences in 


Finally, related to a "can't find repository": 

>>> Making HTTP POST request to https://rb.example.com/api/review-requests/ 
>>> Got API Error 206 (HTTP code 400): The repository path specified is not in 
>>> the list of known repositories. 

In rbtools/commands/post.py, in post_request, there's this: 

# No review_request_id, so we will create a new review request. 
repository = ( 
# 1P DISABLED 2014-06-08 # self.options.repository_url or 
self.options.repository_name or 
self.get_repository_path(repository_info, api_root)) 

Rather than commenting out the first line there per the DISABLED 
comment, I think it would be sufficient to push the repository_name up 
a line, so we'd trust "--repository" a bit more than 

The latter (--repository-url) seems to have two meanings at least vis 
a vis svn: one is "the thing we're going to be calling 'svn info' 
and/or collecting info from" and the second is "a thing we can use to 
identify the repo to RB". The former (--repository) is presumably 
only the name as RB knows it? (Those are guaranteed to be unique, 
We’re putting out 0.6.1 this week. I’ll fix this so that if a repository name 
is specified, then that’s used for identifying the repository on Review Board, 
instead of prioritizing the URL.


One other tiny note -- we've been keeping a patch over 
reviewboard/templates/notifications that removes all the explicit 
"font-size" (e.g. 8px, 9px, 10px) in favor of the occasional 
"font-size: smaller;" or "font-size: larger;". The default font shows 
up hideously tiny in many of the clients we use with these explicit 
sizes, and some browsers behave a lot better re zoom when the size 
isn't explicit in pixels. 
I’d love to see screenshots of how this looks, along with a formal patch on 



Get the Review Board Power Pack at http://www.reviewboard.org/powerpack/ 
Sign up for Review Board hosting at RBCommons: https://rbcommons.com/ 
Happy user? Let us know at http://www.reviewboard.org/users/ 
You received this message because you are subscribed to the Google Groups 
"reviewboard" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard+unsubscr...@googlegroups.com. 
For more options, visit https://groups.google.com/d/optout. 

Get the Review Board Power Pack at http://www.reviewboard.org/powerpack/
Sign up for Review Board hosting at RBCommons: https://rbcommons.com/
Happy user? Let us know at http://www.reviewboard.org/users/
You received this message because you are subscribed to the Google Groups 
"reviewboard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to