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 https://reviews.reviewboard.org/ ===== 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"> <html><head> <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']: 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). We strongly recommend using repository names and not URLs wherever possible. This provides the fastest lookup, and skips the whole issue of differences in URLs. ===== 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?) 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 https://reviews.reviewboard.org/ Christian -- Leander -- 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.
