Perforce proxy servers

[chrisn]

I just filed http://code.google.com/p/reviewboard/issues/detail?id=2182
to describe an issue with ReviewBoard and Perforce proxy servers that
is effectively blocking us from using ReviewBoard.

I'd appreciate any thoughts or BTDT advice about how I might be able
to work around the problem.

Thanks.

[Christian Hammond]

Hi Chris,

The Path field should probably point to the proxy, and Mirror Path should point to 'p4 info'. It's a bit confusing (the Mirror Path name exists for legacy reasons) and I want to clean that up, but start with that. Basically, we'll always communicate through Path, but we check both Path and Mirror Path when using post-review.

We can't log the commands because, with the exception of one case, we're not calling out to p4. We're instead using the Python bindings for the library that is talking directly to the server.

Christian

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

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

[chrisn]

No joy. With Path set to the proxy and Mirror Path to the output of
'p4 info', post-review successfully creates a new review request, but
the server still fails to display diffs. The failure mode is that
patch fails because the files to patch are zero-length due to the
failure to communicate with the server.

On Jul 20, 1:57 pm, Christian Hammond <chip...@chipx86.com> wrote:
> Hi Chris,
>
> The Path field should probably point to the proxy, and Mirror Path should
> point to 'p4 info'. It's a bit confusing (the Mirror Path name exists for
> legacy reasons) and I want to clean that up, but start with that. Basically,
> we'll always communicate through Path, but we check both Path and Mirror
> Path when using post-review.
>
> We can't log the commands because, with the exception of one case, we're not
> calling out to p4. We're instead using the Python bindings for the library
> that is talking directly to the server.
>
> Christian
>
> --

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

>
>
>
>
>
>
>
> On Wed, Jul 20, 2011 at 8:29 AM, chrisn <ch...@newbold.org> wrote:

> > I just filedhttp://code.google.com/p/reviewboard/issues/detail?id=2182

> > to describe an issue with ReviewBoard and Perforce proxy servers that
> > is effectively blocking us from using ReviewBoard.
>
> > I'd appreciate any thoughts or BTDT advice about how I might be able
> > to work around the problem.
>
> > Thanks.
>
> > --
> > 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/

[David Trowbridge]

This configuration is supported with the following:

Set the repository path to the results of "p4 info"
Set the mirror path to the proxy geographically closest to the reviewboard server.

- David

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

[chrisn]

Thanks for the suggestion, David, but it doesn't work either. I've now
tried it both ways around (Path == proxy, Mirror == info and Path ==
info, Mirror =proxy) but the result is the same in either case. post-
review successfully creates the request, but the server is unable to
display diffs. Here's an example error (with depot path information
sanitized):

The patch to '//XXXXX/clear.cpp' didn't apply cleanly. The temporary
files have been left in '/tmp/reviewboard.UVgy2H' for debugging
purposes. `patch` returned: patching file /tmp/reviewboard.UVgy2H/
tmpB2EamP Hunk #1 FAILED at 171. 1 out of 1 hunk FAILED -- saving
rejects to file /tmp/reviewboard.UVgy2H/tmpB2EamP-new.rej

Traceback (most recent call last):
File "/usr/lib/python2.5/site-packages/ReviewBoard-1.5.5-py2.5.egg/
reviewboard/diffviewer/views.py", line 153, in view_diff
interdiffset, highlighting, True)
File "/usr/lib/python2.5/site-packages/ReviewBoard-1.5.5-py2.5.egg/
reviewboard/diffviewer/diffutils.py", line 1066, in get_diff_files
large_data=True)
File "/usr/lib/python2.5/site-packages/Djblets-0.6.7-py2.5.egg/
djblets/util/misc.py", line 166, in cache_memoize
data = lookup_callable()
File "/usr/lib/python2.5/site-packages/ReviewBoard-1.5.5-py2.5.egg/
reviewboard/diffviewer/diffutils.py", line 1065, in <lambda>
enable_syntax_highlighting)),
File "/usr/lib/python2.5/site-packages/ReviewBoard-1.5.5-py2.5.egg/
reviewboard/diffviewer/diffutils.py", line 552, in get_chunks
new = get_patched_file(old, filediff)
File "/usr/lib/python2.5/site-packages/ReviewBoard-1.5.5-py2.5.egg/
reviewboard/diffviewer/diffutils.py", line 374, in get_patched_file
return patch(filediff.diff, buffer, filediff.dest_file)
File "/usr/lib/python2.5/site-packages/ReviewBoard-1.5.5-py2.5.egg/
reviewboard/diffviewer/diffutils.py", line 242, in patch
(filename, tempdir, patch_output))
Exception: The patch to '//XXXXX/clear.cpp' didn't apply cleanly. The
temporary files have been left in '/tmp/reviewboard.UVgy2H' for
debugging purposes.
`patch` returned: patching file /tmp/reviewboard.UVgy2H/tmpB2EamP
Hunk #1 FAILED at 171.
1 out of 1 hunk FAILED -- saving rejects to file /tmp/
reviewboard.UVgy2H/tmpB2EamP-new.rej

When I go and look in the temporary directory, I find a perfectly
valid-looking patch plus empty source and destination files. So the
server is clearly failing to retrieve the contents of the files, but
since there's no useful logging, I really don't have any further
information about what is going wrong. We stumbled into the proxy
issue by looking at Perforce server logs...

-Chris

On Jul 20, 3:31 pm, David Trowbridge <trowb...@gmail.com> wrote:
> This configuration is supported with the following:
>
> Set the repository path to the results of "p4 info"
> Set the mirror path to the proxy geographically closest to the reviewboard server.
>
> - David
>
> On Jul 20, 2011, at 8:29 AM, chrisn <ch...@newbold.org> wrote:
>
>
>
>
>
>
>

> > I just filedhttp://code.google.com/p/reviewboard/issues/detail?id=2182

> > to describe an issue with ReviewBoard and Perforce proxy servers that
> > is effectively blocking us from using ReviewBoard.
>
> > I'd appreciate any thoughts or BTDT advice about how I might be able
> > to work around the problem.
>
> > Thanks.
>
> > --

> > Want to help the Review Board project? Donate today athttp://www.reviewboard.org/donate/
> > Happy user? Let us know athttp://www.reviewboard.org/users/

[SCFrench]

I've been working with Chris on this issue. We seem to have it working
now. There appears to have been two issues that were combining to make
this harder than usual to fix. First, the diagnostics on the call to
p4 in get_file in perforce.py are relying on the exit status to
indicate that something went wrong. However, as I found here:
http://forums.perforce.com/index.php?/topic/682-noob-question-changelists/page__view__findpost__p__1770,
the exit status of p4 is almost always 0. When I finally figured out
what p4 command was being run by get_file, and ran it manually, I got
this failure (edited slightly to remove private information):

% p4 -p perforce-xx-xxx.mathworks.com:1666 -u cnxxxxxx print -q '//xxx/
xxx/queue/matlab/src/m_interpreter/mi_interpreter/clear.cpp#17'
//xxx/xxx/queue/matlab/src/m_interpreter/mi_interpreter/clear.cpp#17 -
no permission for operation on file(s).

% echo $?
0

The link above discusses some ways to improve error checking on
scripted p4 invocations, by using the -s option. Not sure if that will
work for Review Board, but I thought I'd pass it along.

The second issue was that once the file clear.cpp had been incorrectly
retrieved, the bad (empty) version got cached, and subsequent attempts
to fix the problem by changing the name of the repository at the admin
site apparently were effective no-ops because that information is
ignored if the file is already cached locally. Restarting memcached
cleared the cache and allowed us to make progress on the first issue.

Hope this information helps to make Review Board even better!

Scott

On Jul 20, 1:57 pm, Christian Hammond <chip...@chipx86.com> wrote:

> Hi Chris,
>
> The Path field should probably point to the proxy, and Mirror Path should
> point to 'p4 info'. It's a bit confusing (the Mirror Path name exists for
> legacy reasons) and I want to clean that up, but start with that. Basically,
> we'll always communicate through Path, but we check both Path and Mirror
> Path when using post-review.
>
> We can't log the commands because, with the exception of one case, we're not
> calling out to p4. We're instead using the Python bindings for the library
> that is talking directly to the server.
>
> Christian
>
> --

> Christian Hammond - chip...@chipx86.com
> Review Board -http://www.reviewboard.org

> VMware, Inc. -http://www.vmware.com

>
>
>
>
>
>
>
> On Wed, Jul 20, 2011 at 8:29 AM, chrisn <ch...@newbold.org> wrote:

> > I just filedhttp://code.google.com/p/reviewboard/issues/detail?id=2182

> > to describe an issue with ReviewBoard and Perforce proxy servers that
> > is effectively blocking us from using ReviewBoard.
>
> > I'd appreciate any thoughts or BTDT advice about how I might be able
> > to work around the problem.
>
> > Thanks.
>
> > --
> > 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/

[chrisn]

Yes, thanks David and Christian for the tips on configuration that
enabled us to finally track down the mis-configuration. The final,
working configuration has the 'p4 info' name in the repository Path
and the proxy in the Mirror Path.

We still have somewhat of an issue with proxies, however. When the
server to which the proxy directs clients changes, Review Board stops
working until we manually update the configuration with the "new"
value from 'p4 info'. For example, we had things working at the end of
last week, but a weekend fail-over of the Perforce server changed the
result of 'p4 info' and everything stopped working again.

-Chris

On Jul 22, 9:47 am, SCFrench <sc...@mathworks.com> wrote:
> I've been working with Chris on this issue. We seem to have it working
> now. There appears to have been two issues that were combining to make
> this harder than usual to fix. First, the diagnostics on the call to
> p4 in get_file in perforce.py are relying on the exit status to

> indicate that something went wrong. However, as I found here:http://forums.perforce.com/index.php?/topic/682-noob-question-changel...,

[Christian Hammond]

Hi Chris,

You may have said this already and I may have missed it, but is the core problem with the various proxy names that post-review's unable to find the correct repository to patch? If so, you can reference repositories by name instead of by path:

http://www.reviewboard.org/docs/manual/dev/users/tools/post-review/#repository

Christian

--

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

Happy user? Let us know at http://www.reviewboard.org/users/

[chrisn]

Christian--

Sorry if I wasn't clear; yes the problem is with post-review's attempt
to automatically determine the repository. It looks like the
REPOSITORY setting should take care of this issue for us. Now time for
me to RTFM...

Thanks.
-Chris

On Jul 25, 5:05 pm, Christian Hammond <chip...@chipx86.com> wrote:
> Hi Chris,
>

> You may have said this already and I may have missed it, but is the core
> problem with the various proxy names that post-review's unable to find the
> correct repository to patch? If so, you can reference repositories by name
> instead of by path:
>

> http://www.reviewboard.org/docs/manual/dev/users/tools/post-review/#r...

[je...@jeffmarkham.com]

we worked around the issue thusly (see attached perforce.py)

It works well for us without any config issues.