Re: One or more fields had errors: fields: 'path': 'This field is required.'

2009-12-15 Thread Thilo-Alexander Ginkel
On Mon, Dec 14, 2009 at 23:11, Christian Hammond chip...@chipx86.com wrote:
 It should just be the field constraints from those FileFields. We don't do
 any custom validation checks in those forms that I can see.

 It could potentially fail if the diff itself is empty, even though the name
 is populated. Are they uploading using post-review? Might be worth seeing
 what post-review --output-diff shows. Or otherwise, just making sure the
 diff is non-empty.

 Short of that, I wouldn't be able to see off-hand without seeing the code
 and doing my own debugging.

After a lengthy debugging session I think I figured out what is going
on, although the solution still is not 100% clear to me:
- In json.new_diff the request.POST field is completely empty, so
Django is completely right when complaining about the absence of some
fields.
- On the client side, though, the request looks sensible and the MIME
body is correctly built.
- In post-review's http_post method, however, the Content-Length
header is calculated as len(body), which will calculate the length of
the body incorrectly if it contains non-ASCII characters. So, in the
end the sent Content-Length header is shorter than the actual request
body, which will cause Django not to find the terminating MIME
boundary thus ignoring the body.

The question is, how can I calculate the length of this string in
bytes, not characters, taking the wire encoding into account?

 Though it's probably not the cause, it also might be worth checking if there
 are any proxy settings getting in the way. We used to run into some issues
 with users who were going through the proxy for the Review Board server
 (despite it being within the network), and some stuff broke along the way.
 Though I don't believe we hit file upload issues.

I disabled proxies in our custom post-review implementation, so that's
not an issue.

Regards,
Thilo

-- 
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


Re: One or more fields had errors: fields: 'path': 'This field is required.'

2009-12-15 Thread Thilo-Alexander Ginkel
On Tue, Dec 15, 2009 at 15:28, Thilo-Alexander Ginkel th...@ginkel.com wrote:
 The question is, how can I calculate the length of this string in
 bytes, not characters, taking the wire encoding into account?

A patch is available at: http://reviews.reviewboard.org/r/1298/

Regards,
Thilo

-- 
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


Re: One or more fields had errors: fields: 'path': 'This field is required.'

2009-12-15 Thread Ryan Oblak
Regarding this issue on the 1.1 alpha nightlies, I believe it is
caused by the patch I submitted in this review: 
http://reviews.reviewboard.org/r/1295/

It seems as though my fix for the New Review Request web UI broke the
diff upload mechanism.  This affects both the web UI for uploading new
diffs to existing reviews and using post-review to upload new diffs.

I'm going to look into this.  I'll let you know what I find.


On Dec 14, 1:28 pm, Christian Hammond chip...@chipx86.com wrote:
 Which version of Review Board is this running right now? I've seen this
 recently on the 1.1 alpha nightlies.

 Also, what repository type is this? It could indicate a missing dependency.
 The error checking/reporting for that on 1.0.x is pretty bad, but improved
 in 1.1.

 Is this only happening to some people and not everyone?

 Christian

 --
 Christian Hammond - chip...@chipx86.com
 Review Board -http://www.reviewboard.org
 VMware, Inc. -http://www.vmware.com

 On Mon, Dec 14, 2009 at 7:32 AM, Thilo-Alexander Ginkel 
 th...@ginkel.comwrote:



  Hello everyone,

  I am currently somewhat stuck figuring out the cause of an error that
  some colleagues are getting in response to the
  /api/json/reviewrequests/id/diff/new/ call when posting a new review
  request:

  {u'fields': {u'path': [u'This field is required.']}, u'stat': u'fail',
  u'err': {u'msg': u'One or more fields had errors', u'code': 105}}

  I did some debugging on the server side and it seems that in new_diff
  in json.py the form.is_valid() call returns false. I also enhanced
  post-review with some debugging code and can see that the following
  request is dispatched to the server:

   Posting API request: path=api/json/reviewrequests/id/diff/new/,
  fields={}, files={'path': {'content': u'diff', 'filename': 'diff'}}

  Any idea what may be wrong here?

  Thanks,
  Thilo

  --
  Want to help the Review Board project? Donate today at
 http://www.reviewboard.org/donate/
  Happy user? Let us know athttp://www.reviewboard.org/users/
  -~--~~~~--~~--~--~---
  To unsubscribe from this group, send email to
  reviewboard+unsubscr...@googlegroups.comreviewboard%2bunsubscr...@googlegr 
  oups.com
  For more options, visit this group at
 http://groups.google.com/group/reviewboard?hl=en

-- 
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


Re: One or more fields had errors: fields: 'path': 'This field is required.'

2009-12-14 Thread Christian Hammond
Which version of Review Board is this running right now? I've seen this
recently on the 1.1 alpha nightlies.

Also, what repository type is this? It could indicate a missing dependency.
The error checking/reporting for that on 1.0.x is pretty bad, but improved
in 1.1.

Is this only happening to some people and not everyone?

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Mon, Dec 14, 2009 at 7:32 AM, Thilo-Alexander Ginkel th...@ginkel.comwrote:

 Hello everyone,

 I am currently somewhat stuck figuring out the cause of an error that
 some colleagues are getting in response to the
 /api/json/reviewrequests/id/diff/new/ call when posting a new review
 request:

 {u'fields': {u'path': [u'This field is required.']}, u'stat': u'fail',
 u'err': {u'msg': u'One or more fields had errors', u'code': 105}}

 I did some debugging on the server side and it seems that in new_diff
 in json.py the form.is_valid() call returns false. I also enhanced
 post-review with some debugging code and can see that the following
 request is dispatched to the server:

  Posting API request: path=api/json/reviewrequests/id/diff/new/,
 fields={}, files={'path': {'content': u'diff', 'filename': 'diff'}}

 Any idea what may be wrong here?

 Thanks,
 Thilo

 --
 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.comreviewboard%2bunsubscr...@googlegroups.com
 For more options, visit this group at
 http://groups.google.com/group/reviewboard?hl=en

-- 
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

Re: One or more fields had errors: fields: 'path': 'This field is required.'

2009-12-14 Thread Christian Hammond
It should just be the field constraints from those FileFields. We don't do
any custom validation checks in those forms that I can see.

It could potentially fail if the diff itself is empty, even though the name
is populated. Are they uploading using post-review? Might be worth seeing
what post-review --output-diff shows. Or otherwise, just making sure the
diff is non-empty.

Short of that, I wouldn't be able to see off-hand without seeing the code
and doing my own debugging.

Though it's probably not the cause, it also might be worth checking if there
are any proxy settings getting in the way. We used to run into some issues
with users who were going through the proxy for the Review Board server
(despite it being within the network), and some stuff broke along the way.
Though I don't believe we hit file upload issues.

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Mon, Dec 14, 2009 at 1:52 PM, Thilo-Alexander Ginkel th...@ginkel.comwrote:

 On Monday 14 December 2009 22:28:35 Christian Hammond wrote:
  Which version of Review Board is this running right now? I've seen this
  recently on the 1.1 alpha nightlies.

 It's a slightly patched 1.0.5.1.

  Also, what repository type is this? It could indicate a missing
 dependency.
  The error checking/reporting for that on 1.0.x is pretty bad, but
 improved
  in 1.1.

 It's a custom SCM, so I cannot rule out that the bug is in my code.
 Dependencies should not be an issue, though, as the SCM client code is
 mostly
 self-contained and does all of its communication with the SCM via HTTP.

 Do you have an idea what may be causing form.is_valid() to return false? Is
 it
 just the mandatory field constraints, which are being checked? The weird
 thing
 is, that according to my client-side dump the 'path' attribute is populated
 correctly.

  Is this only happening to some people and not everyone?

 Yes.

 Regards,
 Thilo

 --
 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.comreviewboard%2bunsubscr...@googlegroups.com
 For more options, visit this group at
 http://groups.google.com/group/reviewboard?hl=en


-- 
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