Permissions and Offline Mode

2010-12-21 Thread Leonel Togniolli
Hi,

I recently started looking into Review Board and I'm about to give it a try 
with my team. A couple questions before I start:

It appears to be possible as an unauthenticated user to see all review 
quests and the diffs. I need to prevent that and only let logged in users to 
do so. A setting I missed somewhere?

Second, it appears client and server need a live connection to the SCM 
server. I'm using SVN in case that makes a difference. This is inadequate to 
me because we are in a different physical network than our server and reach 
it through on-demand VPN. 

It seems to me that I have all the need, in the client, to create the review 
request. SVN stores the original files, which are used to generate the diffs 
of uncommitted changes, which are uploaded to the server and displayed. This 
is for pre-commit reviews, of course. Otherwise either the client or the 
server would need to reach it and fetch the requested revisions (I'd prefer 
the client but can't be picky about that). Am I missing something?

Otherwise, ReviewBoard looks great. Installation was a little bumpy (on 
windows with apache, mysql) but I figured things out without too much 
trouble. Looking forward to use it for real.

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

Reversed (or previously applied) patch detected! when view diff after git push

2010-12-21 Thread birdsong

I'm getting this error
http://pastebin.com/uEwh1vjR

Setup is:
reviewboard on same servers as bare git repo and cgit server.
path: /home/git/repositories/f.web.git
mirror path: g...@repo.f.com:f.web.git
raw file url mask: 
http://localhost:7000/cgit/f.web/plain/filename?id2=revision

(i know i'm missing the 'gtk+' path component that the docs mention,
but this url works on my version of cgit to retrieve the plain file
and adding gtk+ anywhere in the path broke that)

I am able to view the diff after running git commit and then post-
review.  Once I push my commit to origin, the diff is no longer
available and I get this traceback.

Any ideas?

-- 
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: Permissions and Offline Mode

2010-12-21 Thread Christian Hammond
Hi Leonel,

On Tue, Dec 21, 2010 at 4:20 AM, Leonel Togniolli tognio...@gmail.com wrote:
 Hi,
 I recently started looking into Review Board and I'm about to give it a try
 with my team. A couple questions before I start:
 It appears to be possible as an unauthenticated user to see all review
 quests and the diffs. I need to prevent that and only let logged in users to
 do so. A setting I missed somewhere?

Yes. Admin UI - Settings - Authentication - Allow anonymous access
(or something like that).

In 1.6, you'll have more ability to have fine-grained permissions on
groups and repositories.


 Second, it appears client and server need a live connection to the SCM
 server. I'm using SVN in case that makes a difference. This is inadequate to
 me because we are in a different physical network than our server and reach
 it through on-demand VPN.

This is a requirement. Review Board grabs the files from the SCM and
patches them in order to get the complete original and new files, and
then performs a side-by-side diff. This won't change. Best I can
recommend is coming up with some more persistent tunnel, or moving
where the Review Board server lives.


 It seems to me that I have all the need, in the client, to create the review
 request. SVN stores the original files, which are used to generate the diffs
 of uncommitted changes, which are uploaded to the server and displayed. This
 is for pre-commit reviews, of course. Otherwise either the client or the
 server would need to reach it and fetch the requested revisions (I'd prefer
 the client but can't be picky about that). Am I missing something?

The reason why we don't have the client provide the complete files is
partially for storage reasons. It's way cheaper to store diffs instead
of full files. Otherwise, the databases would absolutely balloon up in
size. As an example, I know a Review Board server in use at one
company that has a database of 19GB, 18GB of which are the diffs for
the hundreds of thousands of review requests (and iterations on them).
Now, if we were to put up entire files, that would grow significantly,
by 10x or more, I'd have to imagine.

Having access to the files from the SCM server gives us flexibility in
how we operate. There was some work being done for Git support where
Review Board could monitor a branch and tell the user if there have
been changes to the branch since the review request. There's a feature
request filed for an extension down the road that could tell you if
the patch still applies to the tip of the tree.

I hope that explains well enough why we're sticking with the method
we're currently using. There are plans for something to help offload
some of this work (interacting with repositories, dealing with the
diff file storage), but the general model won't change.


 Otherwise, ReviewBoard looks great. Installation was a little bumpy (on
 windows with apache, mysql) but I figured things out without too much
 trouble. Looking forward to use it for real.

Windows installs can be tricky. It's far, far easier to install on
Linux. We've had some work done on a Windows installer, which may land
in 1.6, though.

Christian

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


Re: Using reviewboard post-review with 'hg export' ?

2010-12-21 Thread Christian Hammond
Hi,

Sorry for the late reply here. Holidays and all..


On Fri, Dec 17, 2010 at 7:06 AM, Oz Linden (Scott Lawrence)
o...@lindenlab.com wrote:
 Our project uses Mercurial, and so there are a couple of things I'd like to
 be able to have contributors post patches generated with 'hg export' (either
 by uploading or even better with post-review) and have rb extract the
 metadata (description at least) from the export output.

We have some open bugs like this for Git, where the metadata in the
diff (this is in the diff, right?) isn't preserved. Our diff parsers
need some tweaks to not strip out too much. Shouldn't be a lot of
work, but definitely needs to be done.


 Even more important, if they post using export, I'd like to get that
 meta-data back when I download the patch; at present I seem to only get the
 actual diff, which means I can't use 'hg import' to pull in the change.

If we fix our parsers, the stored diffs will retain this data.


 I'm also having a bit of trouble figuring out what the smoothest way to use
 post-review is... the repository configured in the reviewboard server points
 to our canonical project repo on bitbucket:

   https://bitbucket.org/lindenlab/viewer-development/

 but the way that people work is to clone that to other repos either locally
 or on bitbucket, so that url is rarely the one in the hgrc 'default' or
 'default-push' parameter.   Even when it is, post-review fails to match it
 when the trailing slash is missing locally (common).   I think what I'd
 really like is that post-review take a repository name argument somewhere,
 and get the url from the web api...

It's rather important that post-review and Review Board are on the
same page regarding commits. It's fine to have a clone and then
generate a diff using that, so long as either the base of that diff
points to a known commit on the central repository or the base of the
parent diff points to a known commit.

I'm not a Mercurial expert, so I don't have a lot of experience in
using it with Review Board. On Git, we recommend that origin (the
default remote name pointing to a central server) points to the
central repository, and then post-review will use that when
determining the diffs. Maybe something similar can be enforced for
your Mercurial setup?

There's a --repository-url parameter for post-review that can specify
which repository you want to match on the server. It doesn't look like
the Mercurial support in post-review handles this parameter, but it
wouldn't take much to patch that if you wanted to play around with it
and contribute a fix.

The trailing slash thing is a more global problem, outside of
Mercurial. Not just with slashes, but variations on a URL, such as
http/https variations, domain aliases, etc.

I have a couple ideas for fixing this, but haven't really decided
which way to go (maybe all).

One way is to allow a list of aliases to be configured for a
repository, and then match each alias. We'd need to figure out how to
do that query in an optimal way. If the aliases were stored in a
comma-separated list in the database, we'd have to use LIKE
%,alias,% or something, which skips the indexes. So maybe
pre-fetching and caching the list from the database, but that
increases memory usage and may not scale.

Another option is to generate some possible variations, such as
different prefixes, trailing slash, etc., when querying the database.
This would increase the lookup as well, but less so. In most cases,
it'd probably be doing extra work when it doesn't need to, but could
solve this problem in the general cases.

Christian

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


Re: Reversed (or previously applied) patch detected! when view diff after git push

2010-12-21 Thread Christian Hammond
Can you plug in a filename and SHA1 into that raw URL mask and see if
it's actually fetching using the SHA1? If modifying the tip of origin
breaks things, I wonder if it's not actually making use of the SHA1 on
your copy of cgit. If things break with gtk+, maybe there is some
compatibility change.

What you can try doing is browsing your repository, going to a file
(on some older revision), and trying to view the raw file. See what
URL you get. There may be a couple versions of the raw URLs, so you
may have to find the one that uses /plain/.

Christian

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



On Tue, Dec 21, 2010 at 12:31 PM, birdsong david.birds...@gmail.com wrote:

 I'm getting this error
 http://pastebin.com/uEwh1vjR

 Setup is:
 reviewboard on same servers as bare git repo and cgit server.
 path: /home/git/repositories/f.web.git
 mirror path: g...@repo.f.com:f.web.git
 raw file url mask: 
 http://localhost:7000/cgit/f.web/plain/filename?id2=revision

 (i know i'm missing the 'gtk+' path component that the docs mention,
 but this url works on my version of cgit to retrieve the plain file
 and adding gtk+ anywhere in the path broke that)

 I am able to view the diff after running git commit and then post-
 review.  Once I push my commit to origin, the diff is no longer
 available and I get this traceback.

 Any ideas?

 --
 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: Using reviewboard post-review with 'hg export' ?

2010-12-21 Thread Oz Linden (Scott Lawrence)

On 2010-12-21 18:18, Christian Hammond wrote:

There's a --repository-url parameter for post-review that can specify
which repository you want to match on the server. It doesn't look like
the Mercurial support in post-review handles this parameter, but it
wouldn't take much to patch that if you wanted to play around with it
and contribute a fix


I actually was thinking that I'd try to do a patch that adds a 
--repository-name parameter to post-review, which could then use the web 
api to translate the name into the url.   This would avoid the problems 
with trailing slashes and scheme differences, and would match the web UI 
where the poster selects a repo name not a URL.



--
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: Using reviewboard post-review with 'hg export' ?

2010-12-21 Thread Christian Hammond
On Tue, Dec 21, 2010 at 4:50 PM, Oz Linden (Scott Lawrence)
o...@lindenlab.com wrote:
 On 2010-12-21 18:18, Christian Hammond wrote:

 There's a --repository-url parameter for post-review that can specify
 which repository you want to match on the server. It doesn't look like
 the Mercurial support in post-review handles this parameter, but it
 wouldn't take much to patch that if you wanted to play around with it
 and contribute a fix

 I actually was thinking that I'd try to do a patch that adds a
 --repository-name parameter to post-review, which could then use the web api
 to translate the name into the url.   This would avoid the problems with
 trailing slashes and scheme differences, and would match the web UI where
 the poster selects a repo name not a URL.

Yeah, I'd be fine with that. You can probably make use of the
get_repositories call inside post-review to implement this.

If you end up having to do anything with the API, though, then there's
a couple things to note. If not, just consider this an informative
brain dump :)

First, there are two APIs today: the old Review Board 1.0 API, and the
new REST one in RB 1.5.

Today, post-review uses the RB 1.0 API only. I have a chance pending,
which needs some additional work done first, that will update
post-review to use either API, depending on what version of the server
it's talking to.

The old API is going away in 1.6.

So if you did have to touch the API for your needs, then what I'd
recommend is to wait for my new change to land in post-review (ideally
within the week or so -- holidays will undoubtedly get in the way) and
then implement the server-side part using the new API.

Certainly, having some method to quickly query by name on the server
will be more scalable than the get_repositories call, as large servers
with many repositories could result in large amounts of data being
downloaded/parsed, and in the case of the new API, multiple requests
to handle the pagination.

Christian

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


Re: Using reviewboard post-review with 'hg export' ?

2010-12-21 Thread Oz Linden (Scott Lawrence)

On 2010-12-21 20:07, Christian Hammond wrote:

First, there are two APIs today: the old Review Board 1.0 API, and the
new REST one in RB 1.5.

Today, post-review uses the RB 1.0 API only. I have a chance pending,
which needs some additional work done first, that will update
post-review to use either API, depending on what version of the server
it's talking to.


That explains much of what was confusing me...


The old API is going away in 1.6.

So if you did have to touch the API for your needs, then what I'd
recommend is to wait for my new change to land in post-review (ideally
within the week or so -- holidays will undoubtedly get in the way) and
then implement the server-side part using the new API.

Certainly, having some method to quickly query by name on the server
will be more scalable than the get_repositories call, as large servers
with many repositories could result in large amounts of data being
downloaded/parsed, and in the case of the new API, multiple requests
to handle the pagination.


Yes, a direct REST api that returned the url for a name would certainly 
be helpful.


Do you plan to make it possible to require authentication for the REST 
apis in 1.6?  I'm currently running my server with anonymous access 
disabled because enabling anon access made the REST query apis open.



--
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: Using reviewboard post-review with 'hg export' ?

2010-12-21 Thread Christian Hammond
On Tue, Dec 21, 2010 at 6:18 PM, Oz Linden (Scott Lawrence)
o...@lindenlab.com wrote:
 Yes, a direct REST api that returned the url for a name would certainly be
 helpful.

 Do you plan to make it possible to require authentication for the REST apis
 in 1.6?  I'm currently running my server with anonymous access disabled
 because enabling anon access made the REST query apis open.

What parts are you wanting to hide?

I can absolutely see hiding user information (definitely the full
e-mail address, maybe partly the full name) and maybe the group e-mail
address when the user accessing it is anonymous. I don't know that
blocking the entire API is useful, though, if you're allowing
anonymous access to the site anyway.

Christian

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