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.
=
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 \
2349
=
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
htmlhead
title401 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.
=
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']:
continue
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).
=
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.
try:
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
--repository-url.
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,
yes?)
=
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.
--
Leander
--
Get the Review Board Power Pack at
