Re: rbt post handling of 401 and svn --repository-url=file:
On Mon, Jun 9, 2014 at 4:07 PM, Christian Hammond christ...@beanbaginc.com wrote: I should note that we don’t accept pull requests. All patches go through https://reviews.reviewboard.org/ Ah, of course. I saw http://www.reviewboard.org/docs/codebase/dev/getting-started/ shortly after having posted this message, should have read that first. Apologies, of course this would be going through a Review Board instance! In rbtools/api/request.py, in PresetHTTPAuthHandler's http_request method, we comment out self.used = True, [...] 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. I had forgotten, but we set HOME explicitly (to our /data/svn/hooks dir) before going into the rbt post. I can confirm that it has written .python-eggs/ and .rbtools-cookies and .subversion/ and such there, and they look good. I think the issue may be that this our auth layer is also _AROUND_ reviewboard; for various reasons we don't trust any of our services and have folks log in via vhost config before even hitting RB proper: Directory /var/www/rb/htdocs AuthType Basic AuthName ldap AuthBasicProvider ldap AuthLDAPUrl ldap://server.example.com:389/dc=example,dc=com; STARTTLS AuthLDAPGroupAttribute memberUid AuthLDAPGroupAttributeIsDN off Require ldap-group cn=internal,ou=groups,dc=example,dc=com AllowOverride All Options -Indexes +FollowSymLinks Allow from all /Directory Hence needing to always have that Basic auth_header? If it's dropped it won't even get past apache to reviewboard at all. For completeness: What version of Python are you using there? 2.7.5 Do you ever see this problem outside the hook? Yes; I can repro it on the command line from my personal account. FWIW the 0.6.1 drop including the prefer repository name fix seems to have fixed the other 1 or 2 issues! I'm only maintaining the comment-out of self.used = True at the moment. Responses for context, anyway: 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. It seemed like this was the case, which is why I was trying to specify both; if I'm understanding correctly, we must have --repository-url to keep it from assuming a local svn working copy in cwd. 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) [...] I’d love to see screenshots of how this looks, along with a formal patch on https://reviews.reviewboard.org/ Will try to do this over the next few days. =) Thanks for your help! (And thank you and all the other contributors for all the work on Review Board, it has been a great tool for us!) -- 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.
rbt post handling of 401 and svn --repository-url=file:
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
Re: Forcing JSON responses to use https URLs?
On Sat, Sep 17, 2011 at 2:40 PM, Christian Hammond chip...@chipx86.com wrote: [...] We tell Django to build the URLs, and it checks whether HTTPS is used based on the HTTPS environment variable. Maybe there's something you could do there to fake that variable, set it to 1? I need to narrow it down a bit, some of this seems redundant, but I think this fixed it: 1.) SetEnv HTTPS on # in the associated apache config file 2.) os.environ['HTTPS'] = 'on' # in settings_local.py (probably too late?) 3.) a django patch to handlers/modpython.py to remove the ModPythonRequest class is_secure() method try clause, after noticing http://www.mail-archive.com/django-users@googlegroups.com/msg55312.html When I've narrowed it down a bit more I'll try to post a quick reply. I'm also sure my temporary patch to django will get overwritten on the next system update, so I probably need to investigate better ways of accomplishing the same result (and even if it's really necessary). Thanks for the lead. And it seems congratulations are in order for VMware Workstation 8. =) -- Leander -- Want to help the Review Board project? Donate today at http://www.reviewboard.org/donate/ Happy user? Let us know at http://www.reviewboard.org/users/ -~--~~~~--~~--~--~--- To unsubscribe from this group, send email to reviewboard+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/reviewboard?hl=en