How can I debug diff uploading?

[Jonathan Ellis]

I have the server running and I can tool around in the admin
interface. But I'm having trouble with post-review.

post-review says
Error uploading diff: One or more fields had errors (105)
Your review request still exists, but the diff is not attached.

I added these lines before it tries to upload:
print 'request: %s' % review_request # added
print 'content: %s ...' % diff_content.split('\n')[0] # added
try:
server.upload_diff(review_request, diff_content)
...

request: {u'status': u'pending', u'last_updated': u'2008-01-17
16:58:51', u'description': u'', u'repository': {u'path':u'https://
subversion/svn/repos', u'tool': u'Subversion', u'id': 2, u'name':
u'mbxg-test'}, u'id': 10, u'target_groups':[], u'bugs_closed': u'',
u'changenum': None, u'target_people': [], u'testing_done': u'',
u'branch': u'', u'submitter': {u'username': u'root', u'url': u'/users/
root/', u'fullname':u'', u'id': 1, u'email': u'revie...@mbxg.com'},
u'time_added': u'2008-01-17 16:58:51', u'summary': u'', u'public':
False}

content: Index: dt_data_objects/src/com/dtsoft/ws/client/data/pm/
PMPROFG2.java ...

Nothing looks outrageously wrong to my untrained eye (specifically, it
looks like it's diffing correctly). How can I troubleshoot this?
Does 105 correspond to one of the fields in the request?

thanks,

-Jonathan

[Christian Hammond]

What repository is this? It could be that the repository path is incorrect and that it's not able to actually fetch the files correctly.

Also, running post-review with --debug would be useful, as you can see the error output from the server.

Christian

--
Christian Hammond - chi...@chipx86.com
VMware, Inc.

[Jonathan Ellis]

On Jan 17, 6:55 pm, "Christian Hammond" <chip...@chipx86.com> wrote:
> What repository is this?

--debug says Repository info 'Path: https://subversion/svn/repos

This matches svn info:
Repository Root: https://subversion/svn/repos

> It could be that the repository path is incorrect
> and that it's not able to actually fetch the files correctly.

If you mean, it doesn't match a repository on the review board server,
post-update seems to detect that and give an appropriate message. (I
have tested this.)

> Also, running post-review with --debug would be useful, as you can see the
> error output from the server.

Here --debug includes no extra output. Possibly because the server is
throwing back a validation error instead of an internal 500?

-Jonathan

[Jonathan Ellis]

Okay, I figured it out. The problem was an https certificate, but the
error didn't make that clear at all. So here is a patch to print a
more useful error in debug mode:

Index: D:/projects/reviewboard-vendor/contrib/tools/post-review
===================================================================
--- D:/projects/reviewboard-vendor/contrib/tools/post-review (revision
26330)
+++ D:/projects/reviewboard-vendor/contrib/tools/post-review (working
copy)
@@ -174,6 +174,7 @@
rsp = simplejson.loads(data)

if rsp['stat'] == 'fail':
+ debug('failed response is %s' % rsp)
raise APIError, rsp

return rsp


And here is a patch to prevent the problem out-of-the-box, and to
include the traceback in cases of "unexpected errors"

Index: D:/projects/reviewboard/scmtools/svn.py
===================================================================
--- D:/projects/reviewboard/scmtools/svn.py (revision 26350)
+++ D:/projects/reviewboard/scmtools/svn.py (revision 26407)
@@ -19,6 +19,9 @@

import pysvn
self.client = pysvn.Client()
+ def callback_ssl(trust_dict):
+ return True, trust_dict['failures'], False
+ self.client.callback_ssl_server_trust_prompt = callback_ssl
if repository.username:

self.client.set_default_username(str(repository.username))
if repository.password:
@@ -55,11 +58,6 @@
stre = str(e)
if 'File not found' in stre:
raise FileNotFoundError(path, revision, str(e))
- elif 'callback_ssl_server_trust_prompt required' in stre:
- raise SCMError(
- 'HTTPS certificate not accepted. Please ensure
that ' +
- 'the proper certificate exists in ~/.subversion/
auth ' +
- 'for the user that reviewboard is running as.')
else:
raise SCMError(e)

Index: D:/projects/reviewboard/webapi/json.py
===================================================================
--- D:/projects/reviewboard/webapi/json.py (revision 26350)
+++ D:/projects/reviewboard/webapi/json.py (revision 26407)
@@ -1031,12 +1031,13 @@
'path': [str(e)]
}
})
- except Exception, e:
+ except:
+ import sys, traceback
# This could be very wrong, but at least they'll see the
error.
# We probably want a new error type for this.
return JsonResponseError(request, INVALID_FORM_DATA, {
'fields': {
- 'path': [str(e)]
+ 'path': [traceback.format_exception(*sys.exc_info())]
}
})

[Christian Hammond]

Thanks Jonathan.

Would you mind putting this up on our Review Board for discussion?

Christian

[Jonathan Ellis]

Can you add me as a developer? I have a lot of patches and it would
go a lot faster to commit and submit review requests for those, than
create review requests one at a time from an anonymous checkout.

Plus merging from a branch is easier for you than merging from a patch
file. :)

On Jan 18, 2:12 pm, "Christian Hammond" <chip...@chipx86.com> wrote:
> Thanks Jonathan.
>
> Would you mind putting this up on our Review Board for discussion?
>
> Christian
>

> Christian Hammond - chip...@chipx86.com
> VMware, Inc.

[Christian Hammond]

Hi.

We still would prefer reviewing any and all patches before they go in. You'll note that David and I put most of our patches up for review before submitting (unless it's something we've looked at or discussed heavily in person, or something *really* trivial). It's important for us to keep a code review standard for the project, especially given that this is a code review tool, and we need to know what goes in before it goes in.

Christian

On Jan 22, 2008 4:40 PM, Jonathan Ellis <jbe...@gmail.com> wrote:

Can you add me as a developer?  I have a lot of patches and it would
go a lot faster to commit and submit review requests for those, than
create review requests one at a time from an anonymous checkout.

Plus merging from a branch is easier for you than merging from a patch
file. :)