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  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 
> wrote:
>
>
>
> > 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//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//diff/new/,
> > fields={}, files={'path': {'content': u'', '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.com > 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-15 Thread Thilo-Alexander Ginkel
On Tue, Dec 15, 2009 at 15:28, Thilo-Alexander Ginkel  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 Thilo-Alexander Ginkel
On Mon, Dec 14, 2009 at 23:11, Christian Hammond  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-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 wrote:

> 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.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 Thilo-Alexander Ginkel
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.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 wrote:

> 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//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//diff/new/,
> fields={}, files={'path': {'content': u'', '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.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