my aim is to use codereview (http://code.google.com/p/rietveld/) with sympy where we use mercurial. As a first step, two days ago I implemented uploading of base files:
If there are no major objections, my plan is this:
1) fix the server to accept mercurial patches (currently it only accepts svn patches) 2) fix upload.py to upload the base files automatically, for svn, but especially for Mercurial (so far you need to upload the base files by hand using the web interface) 3) fix the remainging issues/problems, polish the patches
Is this the way to go? Any comments/feedback/help is welcome.
I am using bzr for various projects (including, e.g., IPython1), and
suspect that bzr could be supported in a way very similar to
mercurial. I'll spend some time getting up to date with this list and
trying out your patch; in the meantime I am very interested in any
progress you make.
On Tue, May 20, 2008 at 4:08 PM, Ondrej Certik <ond...@certik.cz> wrote:
> Hi,
> my aim is to use codereview (http://code.google.com/p/rietveld/) with > sympy where we use mercurial. As a first step, two days ago I > implemented uploading of base files:
> If there are no major objections, my plan is this:
> 1) fix the server to accept mercurial patches (currently it only > accepts svn patches) > 2) fix upload.py to upload the base files automatically, for svn, but > especially for Mercurial (so far you need to upload the base files by > hand using the web interface) > 3) fix the remainging issues/problems, polish the patches
> Is this the way to go? Any comments/feedback/help is welcome.
On Wed, May 21, 2008 at 7:09 PM, Guido van Rossum <gu...@python.org> wrote:
> Hi Ondrej,
> Great work! This is definitely the way to go.
> I'll try to review your patch in detail ASAP, but I have a few other > commitments this week.
No problem, I am now learning for an exam, so I'll continue on it at the weekend. Also Andi has reviewed the patch now and he has some very good comments.
BTW, I also got a positive feedback on the sympy list, for example:
On Thu, May 22, 2008 at 5:52 AM, Ondrej Certik <ond...@certik.cz> wrote:
> On Wed, May 21, 2008 at 7:09 PM, Guido van Rossum <gu...@python.org> wrote:
>> Hi Ondrej,
>> Great work! This is definitely the way to go.
>> I'll try to review your patch in detail ASAP, but I have a few other >> commitments this week.
> No problem, I am now learning for an exam, so I'll continue on it at > the weekend. Also Andi has reviewed the patch now and he has some very > good comments.
> BTW, I also got a positive feedback on the sympy list, for example:
Well, the whole point of uploading the base rev now is that Rietveld cannot run shell commands at all -- the only things it can do is make HTTP requests. Kirill seems to be missing this (unless you expected me to browse through to later messages).
In the future I'm sure we could speed up the uploads by skipping redundant uploads based on e.g. SHA1 checksum, like App Engine's own appcfg.py does -- but for the first iteration that's a distraction.
> On Thu, May 22, 2008 at 5:52 AM, Ondrej Certik <ond...@certik.cz> wrote:
>> On Wed, May 21, 2008 at 7:09 PM, Guido van Rossum <gu...@python.org> wrote:
>>> Hi Ondrej,
>>> Great work! This is definitely the way to go.
>>> I'll try to review your patch in detail ASAP, but I have a few other >>> commitments this week.
>> No problem, I am now learning for an exam, so I'll continue on it at >> the weekend. Also Andi has reviewed the patch now and he has some very >> good comments.
>> BTW, I also got a positive feedback on the sympy list, for example:
> Well, the whole point of uploading the base rev now is that Rietveld > cannot run shell commands at all -- the only things it can do is make > HTTP requests. Kirill seems to be missing this (unless you expected me > to browse through to later messages).
Yes, but that can be fixed by the current approach of downloading the particular file over http, as it is done for svn. And this works for Mercurial as well.
I think we should support both approaches.
> In the future I'm sure we could speed up the uploads by skipping > redundant uploads based on e.g. SHA1 checksum, like App Engine's own > appcfg.py does -- but for the first iteration that's a distraction.
On Thu, May 22, 2008 at 11:18 AM, Ondrej Certik <ond...@certik.cz> wrote:
>> Well, the whole point of uploading the base rev now is that Rietveld >> cannot run shell commands at all -- the only things it can do is make >> HTTP requests. Kirill seems to be missing this (unless you expected me >> to browse through to later messages).
> Yes, but that can be fixed by the current approach of downloading the > particular file over http, as it is done for svn. And this works for > Mercurial as well.
IIRC, uploading base files was also meant to make Rietveld VCS agnostic. Otherwise, every new VCS requires specific code changes. Not that that is bad but it would require a plugin type architecture.
> Yes, but that can be fixed by the current approach of downloading the > particular file over http, as it is done for svn. And this works for > Mercurial as well.
I don't agree. Uploading the base files has the big advantage that it will open codereview to many more projects, even some crude ones with restricted repositories, obscure VC systems and special or no HTTP access at all. I would prefer the upload feature in upload.py rather than dealing with VC setups on the server side. At least for now...
> On Thu, May 22, 2008 at 11:18 AM, Ondrej Certik <ond...@certik.cz> wrote:
>>> Well, the whole point of uploading the base rev now is that Rietveld >>> cannot run shell commands at all -- the only things it can do is make >>> HTTP requests. Kirill seems to be missing this (unless you expected me >>> to browse through to later messages).
>> Yes, but that can be fixed by the current approach of downloading the >> particular file over http, as it is done for svn. And this works for >> Mercurial as well.
> IIRC, uploading base files was also meant to make Rietveld VCS > agnostic. Otherwise, every new VCS requires specific code changes. Not > that that is bad but it would require a plugin type architecture.
Yes. However both approaches have pros and cons. The huge disadvantage of the base files approach, at least for me, is that it moves the job to upload the files to the upload.py script. However, I really like the web interface, as it is easy for everyone to upload a patch (mercurial changeset). However, having to upload base files over the web by hand is annoying. So if codereview supported mercurial paths, which I am going to implement too, it will be very convenient.
upload.py script is nice, but honestly, I think for newcomers it's easier to just upload the patch over the web.
So unless it causes some maintainability problems, I'd like to support both approaches.
>> Yes, but that can be fixed by the current approach of downloading the >> particular file over http, as it is done for svn. And this works for >> Mercurial as well.
> I don't agree. Uploading the base files has the big advantage that it > will open codereview to many more projects, even some crude ones with > restricted repositories, obscure VC systems and special or no HTTP > access at all. I would prefer the upload feature in upload.py rather > than dealing with VC setups on the server side. At least for now...
> Yes. However both approaches have pros and cons. The huge disadvantage > of the base files approach, at least for me, is that it moves the job > to upload the files to the upload.py script.
In fact, this isn't a disadvantage, but something to do if upload.py should support this.
> However, I really like > the web interface, as it is easy for everyone to upload a patch > (mercurial changeset). However, having to upload base files over the > web by hand is annoying.
It's a matter of design. A form with files included in a patchset on the left and file upload buttons or details about an already uploaded file on the right could be handy.
> upload.py script is nice, but honestly, I think for newcomers it's > easier to just upload the patch over the web.
When first using rietveld, I had some problems with the web form. Don't ask me, I couldn't remember ;-) upload.py gave me some nice defaults for the form fields. Now, before uploading a patchset I do an svn diff to see what's going to be happen, do some changes like "svn revert index.yaml" (again: ;-) and finally I submit the patchset. I definetely prefer upload.py over the web form, the web form was only my first and failed attempt.
Of course, both approaches should be supported. Technically, the upload.py part might be easier to implement since it requires an reletively simple API on the server side and some command line tools of which we can assume that they are installed already. A web-based form to upload base files would only be an interface to that API. Implementing a http fetch solution for various VC systems and setups might be difficult, especially in an restricted Python environment.
>> Yes. However both approaches have pros and cons. The huge disadvantage >> of the base files approach, at least for me, is that it moves the job >> to upload the files to the upload.py script. > In fact, this isn't a disadvantage, but something to do if upload.py > should support this.
Right, I expressed myself not clearly -- I meant the disadvantage is that one basically has to use the upload.py script in this scenario.
>> However, I really like >> the web interface, as it is easy for everyone to upload a patch >> (mercurial changeset). However, having to upload base files over the >> web by hand is annoying. > It's a matter of design. A form with files included in a patchset on > the left and file upload buttons or details about an already uploaded > file on the right could be handy.
Yes, that's one improvement on my patch. However, that doesn't really help, because if I change 10 files, then I need to manually upload ten files (no matter I can use just one form). Also actually getting the original files (either from svn or hg) by hand is annoying, as typically the files that are lying on my disk are the changed ones.
I think one is basically forced to use the upload.py, that can make all of this automatic.
>> upload.py script is nice, but honestly, I think for newcomers it's >> easier to just upload the patch over the web. > When first using rietveld, I had some problems with the web form. > Don't ask me, I couldn't remember ;-) upload.py gave me some nice > defaults for the form fields.
OK. I have the exact opposite experience, the web interface was clear and it was easy for me to upload a patch. The upload.py script was more difficult for me to get going and understand.
> Now, before uploading a patchset I do an svn diff to see what's going > to be happen, do some changes like "svn revert index.yaml" (again: ;-) > and finally I submit the patchset. I definetely prefer upload.py over > the web form, the web form was only my first and failed attempt.
> Of course, both approaches should be supported. Technically, the > upload.py part might be easier to implement since it requires an > reletively simple API on the server side and some command line tools > of which we can assume that they are installed already. A web-based > form to upload base files would only be an interface to that API. > Implementing a http fetch solution for various VC systems and setups > might be difficult, especially in an restricted Python environment.
Actually, I think it's not the case -- making codereview download files from mercurial over http was just a matter of changing one line with the address. Now I only need to implement revisions and supporting both svn and hg, but that should be easy.
Next step then will be to fix upload.py to upload base files, but that will be quite some work, i.e. the 1MB limit, server changes, etc.
But if you disagree, let's split the work: you implement the upload.py improvoements (it is easy for you) and I'll implement the url fetching (easy for me). :)
Without going through a line-by-line response, I want to strongly recommend moving the responsibility of uploading the base files completely to upload.py; IOW a push model instead of a pull model. Some reasons to favor a push model:
- There are many configurations where the repository is not accessible via HTTP. There could be a firewall, the repository could be down temporarily, accessing it could be really slow, there could be an authentication requirement (I really don't want to store passwords or private keys in Rietveld), the URL scheme could be hard to guess, the protocol could be something else than HTTP, etc. The developer uploading the patch *by defition* has access to the base version so none of these issues apply there.
- If Rietveld ever has to access a repository while the user is waiting for the response to a request, the user will have to wait for the repository access to complete, which adds extra waiting and the potential of exceeding the hard 10-second limit on request handlers in App Engine.
- Someone who wants to support a new style of repository doesn't have to convince the Rietveld developers to add support for their repository; they can provide their own modified version of upload.py (it's open source!).
Trust me. Even though I originally wrote Rietveld using a pull model (mostly because it was easier to get started that way), trying to make it work for a variety of SVN repositories has already convinced me of the superiority of a push model. Moreover, I've received feedback from the current Mondrian tech lead at Google indicating that the pull model is one of the major scalability issues with Mondrian.
> Without going through a line-by-line response, I want to strongly > recommend moving the responsibility of uploading the base files > completely to upload.py; IOW a push model instead of a pull model. > Some reasons to favor a push model:
> - There are many configurations where the repository is not accessible > via HTTP. There could be a firewall, the repository could be down > temporarily, accessing it could be really slow, there could be an > authentication requirement (I really don't want to store passwords or > private keys in Rietveld), the URL scheme could be hard to guess, the > protocol could be something else than HTTP, etc. The developer > uploading the patch *by defition* has access to the base version so > none of these issues apply there.
> - If Rietveld ever has to access a repository while the user is > waiting for the response to a request, the user will have to wait for > the repository access to complete, which adds extra waiting and the > potential of exceeding the hard 10-second limit on request handlers in > App Engine.
> - Someone who wants to support a new style of repository doesn't have > to convince the Rietveld developers to add support for their > repository; they can provide their own modified version of upload.py > (it's open source!).
> Trust me. Even though I originally wrote Rietveld using a pull model > (mostly because it was easier to get started that way), trying to make > it work for a variety of SVN repositories has already convinced me of > the superiority of a push model. Moreover, I've received feedback from > the current Mondrian tech lead at Google indicating that the pull > model is one of the major scalability issues with Mondrian.
Those are good arguments. I think the best way is that I simply try to implement what I want or think it's good and then we'll discuss it over each particular patchset, if it's good/maintainable/worthy and can go in, or it works, but it's unmaintainable/unscalable.
Ondrej, an API to upload multiple bases for a patchset along with a upload form would be an great improvement to rietveld. I can modify upload.py to use this API for submitting base files next week.
> Ondrej, an API to upload multiple bases for a patchset along with a > upload form would be an great improvement to rietveld. I can modify > upload.py to use this API for submitting base files next week.
I'll try to do it this weekend, so that we can start using rietveld for sympy now. Then we can try to polish the patch, get it in, improve it, etc.
On May 21, 1:08 am, "Ondrej Certik" <ond...@certik.cz> wrote:
> Hi,
> my aim is to use codereview (http://code.google.com/p/rietveld/) with
> sympy where we use mercurial. As a first step, two days ago I
> implemented uploading of base files:
> I would appreciate very much any feedback/review from anyone
> interested in these features.
Do you know if it's possible to use rietveld to review patch series?
(A patch series would be a group of patches, each building upon the
one before it.) Many people use Mercurial queues to manage patch
series instead of single patches to keep individual patches properly
focused. A good way to review such beasts would be very welcome.
If so, I think it would be great to add support for uploading entire
Mercurial patch queues to rietveld. (And also quilt patch sets.)
If not, does anyone have an idea how hard it would be to add this?
On Fri, May 23, 2008 at 8:19 AM, parren <peter.arrenbre...@gmail.com> wrote:
> On May 21, 1:08 am, "Ondrej Certik" <ond...@certik.cz> wrote: >> Hi,
>> my aim is to use codereview (http://code.google.com/p/rietveld/) with >> sympy where we use mercurial. As a first step, two days ago I >> implemented uploading of base files:
>> I would appreciate very much any feedback/review from anyone >> interested in these features.
> Do you know if it's possible to use rietveld to review patch series? > (A patch series would be a group of patches, each building upon the > one before it.) Many people use Mercurial queues to manage patch > series instead of single patches to keep individual patches properly > focused. A good way to review such beasts would be very welcome.
> If so, I think it would be great to add support for uploading entire > Mercurial patch queues to rietveld. (And also quilt patch sets.)
> If not, does anyone have an idea how hard it would be to add this?
Yes, indeed, it will be possible, by uploading the original (base) files together with the patch. Then you can upload as many patches as you want, no matter which VCS or revision you are using.
See the rietveld mailinglist and my patches for more info. I'll try to finish some basic functionality over the weekend, so that we can start using rietveld with sympy now. And I hope other people will then join in and polish it/improve it.
On Fri, May 23, 2008 at 8:57 AM, Ondrej Certik <ond...@certik.cz> wrote:
> On Fri, May 23, 2008 at 8:19 AM, parren <peter.arrenbre...@gmail.com> wrote:
>> On May 21, 1:08 am, "Ondrej Certik" <ond...@certik.cz> wrote: >>> Hi,
>>> my aim is to use codereview (http://code.google.com/p/rietveld/) with >>> sympy where we use mercurial. As a first step, two days ago I >>> implemented uploading of base files:
>>> I would appreciate very much any feedback/review from anyone >>> interested in these features.
>> Do you know if it's possible to use rietveld to review patch series? >> (A patch series would be a group of patches, each building upon the >> one before it.) Many people use Mercurial queues to manage patch >> series instead of single patches to keep individual patches properly >> focused. A good way to review such beasts would be very welcome.
>> If so, I think it would be great to add support for uploading entire >> Mercurial patch queues to rietveld. (And also quilt patch sets.)
>> If not, does anyone have an idea how hard it would be to add this?
> Yes, indeed, it will be possible, by uploading the original (base) > files together with the patch. > Then you can upload as many patches as you want, no matter which VCS > or revision you are using.
> See the rietveld mailinglist and my patches for more info. I'll try to > finish some basic functionality over the weekend, so that we can > start using rietveld with sympy now. And I hope other people will then > join in and polish it/improve it.
Great! I guess this could be automated rather nicely to upload a patch series and send out an email with links to all the individual patches for review. Then the email-message would serve to navigate the patches. (I'm assuming rietveld does not have a built-in concept of related patches, or does it?)
I'll keep tabs on your progress and see if I can help with this once your stuff is ready.
On Thu, May 22, 2008 at 09:33:38AM -0700, Guido van Rossum wrote:
> Without going through a line-by-line response, I want to strongly > recommend moving the responsibility of uploading the base files > completely to upload.py; IOW a push model instead of a pull model. > Some reasons to favor a push model:
Why can't we have it both ways?
e.g.
- having a mechanism to retrieve base files and other possibly needed information from the repository itself (e.g. over http), and
- fallback to manual upload of base files, if such mechanism is not available?
Yes, this means plugin architecture, but still rietveld will be VCS agnostic because the fallback is there.
> - There are many configurations where the repository is not accessible > via HTTP. There could be a firewall, the repository could be down > temporarily, accessing it could be really slow, there could be an > authentication requirement (I really don't want to store passwords or > private keys in Rietveld), the URL scheme could be hard to guess, the > protocol could be something else than HTTP, etc. The developer > uploading the patch *by defition* has access to the base version so > none of these issues apply there.
Besides "URL scheme is hard to guess", which should be the work of a VCS-support plugin, this arguments means that sometimes you need to fallback to manual upload, that's all.
As to slow access, please read below.
> - If Rietveld ever has to access a repository while the user is > waiting for the response to a request, the user will have to wait for > the repository access to complete, which adds extra waiting and the > potential of exceeding the hard 10-second limit on request handlers in > App Engine.
That could be true.
But can't rietveld access the repository when a patch is _initially_ uploaded?
If the files have to be uploaded, shouldn't it make a big difference time-wise where to receive them from?
So I think an option maybe makes sense here at which stage rietveld should fetch this files:
- if you have fast access to repository, accessing base files could be made lazy, or - if you have not-so-fast link to it, and the repository is sometimes down, let's load everything in the first place.
And anyway, rietveld have to download a file only once -- the first time it is downloaded it could be stored in the DB for later access.
> - Someone who wants to support a new style of repository doesn't have > to convince the Rietveld developers to add support for their > repository; they can provide their own modified version of upload.py > (it's open source!).
Fallback to manual upload works here.
> Trust me. Even though I originally wrote Rietveld using a pull model > (mostly because it was easier to get started that way), trying to make > it work for a variety of SVN repositories has already convinced me of > the superiority of a push model.
Maybe this is true for SVN, I can't say.
> Moreover, I've received feedback from > the current Mondrian tech lead at Google indicating that the pull > model is one of the major scalability issues with Mondrian.
Hmm, I think we anyway use the "push" model because we "push" patches upstream to rietveld :)
I think everything else could be made the same performance wise - see my comments about the stage at which to upload, and caching in the DB.
So to me this boils down to what is more convenient and easier to use for _humans_:
upload only a patch is:
- less work - enough information to restore everything else from parent hash - one could not mess with lying to reitveld by uploading already modified (by accident, or by intention) base files.
However this are all only words - I can't say or prove anything with code -- I'm under deep time pressure at present.
-- Всего хорошего, Кирилл.
P.S.
And the funny thing about Mercurial or other dirstributed VCS is that you could setup several "main" repositories, and "load-balance" fetching information from them :)
> On Thu, May 22, 2008 at 09:33:38AM -0700, Guido van Rossum wrote:
>> Without going through a line-by-line response, I want to strongly >> recommend moving the responsibility of uploading the base files >> completely to upload.py; IOW a push model instead of a pull model. >> Some reasons to favor a push model:
> Why can't we have it both ways?
> e.g.
> - having a mechanism to retrieve base files and other possibly needed > information from the repository itself (e.g. over http), and
> - fallback to manual upload of base files, if such mechanism is not > available?
> Yes, this means plugin architecture, but still rietveld will be VCS > agnostic because the fallback is there.
>> - There are many configurations where the repository is not accessible >> via HTTP. There could be a firewall, the repository could be down >> temporarily, accessing it could be really slow, there could be an >> authentication requirement (I really don't want to store passwords or >> private keys in Rietveld), the URL scheme could be hard to guess, the >> protocol could be something else than HTTP, etc. The developer >> uploading the patch *by defition* has access to the base version so >> none of these issues apply there.
> Besides "URL scheme is hard to guess", which should be the work of a > VCS-support plugin, this arguments means that sometimes you need to > fallback to manual upload, that's all.
> As to slow access, please read below.
>> - If Rietveld ever has to access a repository while the user is >> waiting for the response to a request, the user will have to wait for >> the repository access to complete, which adds extra waiting and the >> potential of exceeding the hard 10-second limit on request handlers in >> App Engine.
> That could be true.
> But can't rietveld access the repository when a patch is _initially_ > uploaded?
> If the files have to be uploaded, shouldn't it make a big difference > time-wise where to receive them from?
> So I think an option maybe makes sense here at which stage rietveld > should fetch this files:
> - if you have fast access to repository, accessing base files could be > made lazy, or > - if you have not-so-fast link to it, and the repository is sometimes > down, let's load everything in the first place.
> And anyway, rietveld have to download a file only once -- the first time > it is downloaded it could be stored in the DB for later access.
>> - Someone who wants to support a new style of repository doesn't have >> to convince the Rietveld developers to add support for their >> repository; they can provide their own modified version of upload.py >> (it's open source!).
> Fallback to manual upload works here.
>> Trust me. Even though I originally wrote Rietveld using a pull model >> (mostly because it was easier to get started that way), trying to make >> it work for a variety of SVN repositories has already convinced me of >> the superiority of a push model.
> Maybe this is true for SVN, I can't say.
>> Moreover, I've received feedback from >> the current Mondrian tech lead at Google indicating that the pull >> model is one of the major scalability issues with Mondrian.
> Hmm, I think we anyway use the "push" model because we "push" patches > upstream to rietveld :)
> I think everything else could be made the same performance wise - see my > comments about the stage at which to upload, and caching in the DB.
> So to me this boils down to what is more convenient and easier to use > for _humans_:
> upload only a patch is:
> - less work > - enough information to restore everything else from parent hash > - one could not mess with lying to reitveld by uploading already > modified (by accident, or by intention) base files.
> However this are all only words - I can't say or prove anything with > code -- I'm under deep time pressure at present.
Thanks very much for taking part in the discussion. I'll try to prove the points with a code. :)
One problem with fetching the files as part of the upload request is that if there are many files you will run into either the 10 second request time limit or the 1 MB urlfetch limit. With the push model, upload.py can make many requests so this is less of a problem.
> On Thu, May 22, 2008 at 09:33:38AM -0700, Guido van Rossum wrote:
>> Without going through a line-by-line response, I want to strongly >> recommend moving the responsibility of uploading the base files >> completely to upload.py; IOW a push model instead of a pull model. >> Some reasons to favor a push model:
> Why can't we have it both ways?
> e.g.
> - having a mechanism to retrieve base files and other possibly needed > information from the repository itself (e.g. over http), and
> - fallback to manual upload of base files, if such mechanism is not > available?
> Yes, this means plugin architecture, but still rietveld will be VCS > agnostic because the fallback is there.
>> - There are many configurations where the repository is not accessible >> via HTTP. There could be a firewall, the repository could be down >> temporarily, accessing it could be really slow, there could be an >> authentication requirement (I really don't want to store passwords or >> private keys in Rietveld), the URL scheme could be hard to guess, the >> protocol could be something else than HTTP, etc. The developer >> uploading the patch *by defition* has access to the base version so >> none of these issues apply there.
> Besides "URL scheme is hard to guess", which should be the work of a > VCS-support plugin, this arguments means that sometimes you need to > fallback to manual upload, that's all.
> As to slow access, please read below.
>> - If Rietveld ever has to access a repository while the user is >> waiting for the response to a request, the user will have to wait for >> the repository access to complete, which adds extra waiting and the >> potential of exceeding the hard 10-second limit on request handlers in >> App Engine.
> That could be true.
> But can't rietveld access the repository when a patch is _initially_ > uploaded?
> If the files have to be uploaded, shouldn't it make a big difference > time-wise where to receive them from?
> So I think an option maybe makes sense here at which stage rietveld > should fetch this files:
> - if you have fast access to repository, accessing base files could be > made lazy, or > - if you have not-so-fast link to it, and the repository is sometimes > down, let's load everything in the first place.
> And anyway, rietveld have to download a file only once -- the first time > it is downloaded it could be stored in the DB for later access.
>> - Someone who wants to support a new style of repository doesn't have >> to convince the Rietveld developers to add support for their >> repository; they can provide their own modified version of upload.py >> (it's open source!).
> Fallback to manual upload works here.
>> Trust me. Even though I originally wrote Rietveld using a pull model >> (mostly because it was easier to get started that way), trying to make >> it work for a variety of SVN repositories has already convinced me of >> the superiority of a push model.
> Maybe this is true for SVN, I can't say.
>> Moreover, I've received feedback from >> the current Mondrian tech lead at Google indicating that the pull >> model is one of the major scalability issues with Mondrian.
> Hmm, I think we anyway use the "push" model because we "push" patches > upstream to rietveld :)
> I think everything else could be made the same performance wise - see my > comments about the stage at which to upload, and caching in the DB.
> So to me this boils down to what is more convenient and easier to use > for _humans_:
> upload only a patch is:
> - less work > - enough information to restore everything else from parent hash > - one could not mess with lying to reitveld by uploading already > modified (by accident, or by intention) base files.
> However this are all only words - I can't say or prove anything with > code -- I'm under deep time pressure at present.
> -- > Всего хорошего, Кирилл.
> P.S.
> And the funny thing about Mercurial or other dirstributed VCS is that > you could setup several "main" repositories, and "load-balance" fetching > information from them :)
I've just uploaded a patch on codereview (http://
codereview.appspot.com/1501) that focusses on pushing files (incl.
>1MB) from a subversion repository to codereview using upload.py. It's
working but not finished right now:
* more exceptions must be raised in different cases (e.g. accessing
content while upload in progress...)
* some exceptions must be catched
* upload.py should not upload binary files
* a MAX_UPLOAD_SIZE should be implemented (both on client and server
side)
* issue_owner_required decorator needs to be implemented
* integration with Ondrej's patch
I'm interested in your suggestions and how and if we could integrate
it with Ondrej's patch.
> I've just uploaded a patch on codereview (http:// > codereview.appspot.com/1501) that focusses on pushing files (incl. >>1MB) from a subversion repository to codereview using upload.py. It's > working but not finished right now:
> * more exceptions must be raised in different cases (e.g. accessing > content while upload in progress...) > * some exceptions must be catched > * upload.py should not upload binary files > * a MAX_UPLOAD_SIZE should be implemented (both on client and server > side) > * issue_owner_required decorator needs to be implemented > * integration with Ondrej's patch
> I'm interested in your suggestions and how and if we could integrate > it with Ondrej's patch.
Hi Andi,
could you please integrate it with my patch and/or finish your patch and then take over my patch? I am unfortunately quite busy this week, so if you have time, please go ahead. Otherwise, it'd be nice if we could merge it somehow, but I think it shouldn't be much work, my patch is essentialy just fixing upload.py to use "hg" instead of "svn" and then fixing the server part to understand Mercurial's format of patches. The best thing would be if we could all agree on something, get it in and then look at the rest and get it in as well. Be it my or your patch as the first one, it doesn't matter.
On Mon, Jun 2, 2008 at 11:46 AM, Ondrej Certik <ond...@certik.cz> wrote: > On Sat, May 31, 2008 at 6:58 PM, Andi Albrecht > <albrecht.a...@googlemail.com> wrote:
>> I've just uploaded a patch on codereview (http:// >> codereview.appspot.com/1501) that focusses on pushing files (incl. >>>1MB) from a subversion repository to codereview using upload.py. It's >> working but not finished right now:
>> * more exceptions must be raised in different cases (e.g. accessing >> content while upload in progress...) >> * some exceptions must be catched >> * upload.py should not upload binary files >> * a MAX_UPLOAD_SIZE should be implemented (both on client and server >> side) >> * issue_owner_required decorator needs to be implemented >> * integration with Ondrej's patch
>> I'm interested in your suggestions and how and if we could integrate >> it with Ondrej's patch.
> Hi Andi,
> could you please integrate it with my patch and/or finish your patch > and then take over my patch? I am unfortunately quite busy this week, > so if you have time, please go ahead. Otherwise, it'd be nice if we > could merge it somehow, but I think it shouldn't be much work, my > patch is essentialy just fixing upload.py to use "hg" instead of "svn" > and then fixing the server part to understand Mercurial's format of > patches. The best thing would be if we could all agree on something, > get it in and then look at the rest and get it in as well. Be it my or > your patch as the first one, it doesn't matter.
Hi Andi and Guido,
let's get things moving again, I reviewed Andi's patch and I think it is ok to go in after some minor improvements. Also I think it is a good base for my patch, as it basically does the same thing, only using <1MB chunks. If Guido is busy and you have time Andi, we can setup a temporary repository and push the patches in, test it, play with it and when Guido gets some time, he can pull the changes to the official repo.
On Wed, Jun 4, 2008 at 8:52 AM, Ondrej Certik <ond...@certik.cz> wrote:
> On Mon, Jun 2, 2008 at 11:46 AM, Ondrej Certik <ond...@certik.cz> wrote: >> On Sat, May 31, 2008 at 6:58 PM, Andi Albrecht >> <albrecht.a...@googlemail.com> wrote:
>>> I've just uploaded a patch on codereview (http:// >>> codereview.appspot.com/1501) that focusses on pushing files (incl. >>>>1MB) from a subversion repository to codereview using upload.py. It's >>> working but not finished right now:
>>> * more exceptions must be raised in different cases (e.g. accessing >>> content while upload in progress...) >>> * some exceptions must be catched >>> * upload.py should not upload binary files >>> * a MAX_UPLOAD_SIZE should be implemented (both on client and server >>> side) >>> * issue_owner_required decorator needs to be implemented >>> * integration with Ondrej's patch
>>> I'm interested in your suggestions and how and if we could integrate >>> it with Ondrej's patch.
>> Hi Andi,
>> could you please integrate it with my patch and/or finish your patch >> and then take over my patch? I am unfortunately quite busy this week, >> so if you have time, please go ahead. Otherwise, it'd be nice if we >> could merge it somehow, but I think it shouldn't be much work, my >> patch is essentialy just fixing upload.py to use "hg" instead of "svn" >> and then fixing the server part to understand Mercurial's format of >> patches. The best thing would be if we could all agree on something, >> get it in and then look at the rest and get it in as well. Be it my or >> your patch as the first one, it doesn't matter.
> Hi Andi and Guido,
> let's get things moving again, I reviewed Andi's patch and I think it > is ok to go in after some minor improvements. Also I think it is a > good base for my patch, as it basically does the same thing, only > using <1MB chunks. If Guido is busy and you have time Andi, we can > setup a temporary repository and push the patches in, test it, play > with it and when Guido gets some time, he can pull the changes to the > official repo.
