Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Tue, Jun 03, 2008 at 08:04:13AM -0500, Shawn Walker wrote: > Alright, I'll make that change later tonight and then assume I can putback? I don't think I have anything left, sure. :) Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/6/3 Danek Duvall <[EMAIL PROTECTED]>: > Looks like you need to remove the "perl-like" return from updatelog.py, > too. I think it's probably fine otherwise. Yes, I know. I just got into the habit of doing an explicit return to avoid having the last statement evaluation pushed onto the stack. One of perl's more "endearing" quirks. Alright, I'll make that change later tonight and then assume I can putback? Cheers, -- Shawn Walker ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Mon, Jun 02, 2008 at 11:06:13PM -0500, Shawn Walker wrote: > > 142 doesn't fire? Is there any point in the code in this method from line > > Because of line 121, 142 should always fire if we get there. > > As a result, I have removed the op_method and "if" condition. The rest > should still do the right thing. Okay. Looks much saner now. Though I think that we don't have to assume that the version isn't supported -- we can check explicitly. And give a "what the hell?" error if that's not the case. > > I'd probably go one step further and put the cherrypy build in its own > > target as well. Right now, it's going to rebuild cherrypy every single > > time you type make install. I'm not too annoyed by this yet, though, > > so maybe I'll be nice and fix it myself sometime later. > > I did that on purpose (due to how often I was doing make clobber and > trying to get things right during development), but I'm not opposed to > changing it. Though once you've gotten it right, you don't need as much of the "prevent me from doing silly things" in a makefile like this. At least IMO. > I've got it split up now so that it won't build the package for > CherryPy or install CherryPy unless the expected files are missing. Okay, that's probably fine. I can diddle with it if it bugs me enough. Looks like you need to remove the "perl-like" return from updatelog.py, too. I think it's probably fine otherwise. Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/6/2 Danek Duvall <[EMAIL PROTECTED]>: > On Sun, Jun 01, 2008 at 10:27:13PM -0500, Shawn Walker wrote: > >> http://cr.opensolaris.org/~swalker/pkg-depot-9/ > > catalog.py: > > - line 1167: why the return statement here? Perl habits die hard :-) Changed. > repository.py: > > - line 90: you'll end up with two adjacent spaces here. Fixed. > - line 101: capitalize first word of sentence. Fixed. >> > - line 141: I'm confused on how we might get here. It seems like no >> >matter what you do, unless you get shunted into the face code above, >> >we're going to error out of this method. >> >> I think you're right; it looks like a leftover from the old logic. > > I'm still confused here. If we don't go into the face code, does this > method do anything useful other than return errors? What happens if line No. It is strictly for error handling. > 142 doesn't fire? Is there any point in the code in this method from line Because of line 121, 142 should always fire if we get there. As a result, I have removed the op_method and "if" condition. The rest should still do the right thing. > 131 onwards? Yes, there is. If you request: http://localhost:1/search/1/ ...it will return a message telling you that version 1 is not supported for search. Whereas if you request: http://localhost:1/foo/1/ You'll get face.unknown(). > pkginfo: > > - line 28: Huh? I suppose you could simply say "all" here, but we >typically don't tag packages as being architecture neutral, even if >they are. Sorry, I wasn't aware of that. CherryPy is architecture neutral. I had gotten the "ANY" value from a pkginfo I found on src.opensolaris.org. However, I have changed this to "ISA" as it was previously. >> > SUNWpython-cherrypy/Makefile: >> > >> > - If nothing else, your "prototype" target rule should be broken up into >> >more rules. >> >> I'm not sure what you wanted, but I have split it up and simplified it >> some more. > > That's better. I'd probably go one step further and put the cherrypy build > in its own target as well. Right now, it's going to rebuild cherrypy every > single time you type make install. I'm not too annoyed by this yet, though, > so maybe I'll be nice and fix it myself sometime later. I did that on purpose (due to how often I was doing make clobber and trying to get things right during development), but I'm not opposed to changing it. I've got it split up now so that it won't build the package for CherryPy or install CherryPy unless the expected files are missing. I also fixed a few warnings I was getting from pkgmk because I missed three or four directories in the prototype (oops). I hope the new version is better? Updated webrev: http://cr.opensolaris.org/~swalker/pkg-depot-10/ Diff since last webrev: http://cr.opensolaris.org/~swalker/pkg-depot-10/raw_files/pkg-depot-10.patch Thanks for your patience, I would prefer to get this as close to right the first time as possible. Cheers, -- Shawn Walker ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Yes, billions and billions served off just a quarter-pound of beef. I'm assuming you mean there's no sensible alternative for running the depot without CherryPy, in which case I agree. As you previously noted, the eventual default install should give us both CherryPy and libbe. These exception handling blocks are largely a convenience to the developer and owner of a misconfigured system. Perhaps there's some way to catch exceptions for any missing dependency and print that in a generic fashion. That's certainly outside the scope of this change, but it would prevent us from having to write a one-off exception handler for every dependency that we introduce. -j On Mon, Jun 02, 2008 at 01:48:08PM -0700, Danek Duvall wrote: > On Mon, Jun 02, 2008 at 01:45:17PM -0700, [EMAIL PROTECTED] wrote: > > > I thought we had one on libbe. I was under the impression that we had > > similar logic around that import too. > > Ah, yes. Though there we just pass if the import fails, but we have > special sauce to make it work without libbe, which doesn't make sense here. > > Danek > ___ > pkg-discuss mailing list > pkg-discuss@opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Mon, Jun 02, 2008 at 01:46:51PM -0500, Shawn Walker wrote: > > Okay (I skipped most of that subthread, since I was away). It's not clear > > to me that this is useful -- on a system that hasn't been hacked to bits, > > it'll be there (otherwise SUNWipkg won't even install). I'll want this to > > go away, though, certainly once cherrypy gets properly integrated. > > I have no objections to removing the check completely if so desired. > > Just let me know. No, leave it in. > In the case of transaction.py, the open() function is returning a > particular status to the calling function. > > The calling function is explicitly evaluating the return value and > then performing exceptions or various other bits of logic based on > that return value. > > As such, it wasn't appropriate for me to raise an exception there. Okay, sorry I didn't pick up on that. > So, in short, it's just doing what it used to. My personal preference > is to let the caller handle abandon() and return the status since that > would make it match open()'s behaviour. > > Would that be your desired approach? I don't really care too much; if you think that'd be cleaner ... > >> > SUNWpython-cherrypy/Makefile: > >> > > >> > - I wonder if perhaps doing the tarball download higher up, in the main > >> >"make install" wouldn't be better -- so that it'll work out of the > >> > box, > >> >and you could integrate it into "make link" and so on. > >> > >> Eventually cherrypy will go away and not be part of the build, so I'm > >> not certain. > > > > Yeah, but we don't have a timeframe on that yet, do we? Seems like we > > could be in the current situation for a while. > > True. So what are your expectations then in regards to this wad? Don't worry about it for now. If it bugs me enough, I'll do something about it. Thanks, Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Mon, Jun 02, 2008 at 01:45:17PM -0700, [EMAIL PROTECTED] wrote: > I thought we had one on libbe. I was under the impression that we had > similar logic around that import too. Ah, yes. Though there we just pass if the import fails, but we have special sauce to make it work without libbe, which doesn't make sense here. Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
I thought we had one on libbe. I was under the impression that we had similar logic around that import too. -j On Mon, Jun 02, 2008 at 11:44:53AM -0700, Danek Duvall wrote: > On Mon, Jun 02, 2008 at 11:39:54AM -0700, [EMAIL PROTECTED] wrote: > > > This would seem to violate the principle of least surprise. If we're > > able to determine that a component that isn't part of the default runtime > > is missing, it's better to emit a useful message instead of a traceback. > > Fair enough. We don't have any other such dependencies, do we? > > Danek > ___ > pkg-discuss mailing list > pkg-discuss@opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/6/2 Danek Duvall <[EMAIL PROTECTED]>: > On Sun, Jun 01, 2008 at 10:27:13PM -0500, Shawn Walker wrote: > >> > - line 72: you're not actually doing a version check here. Is there a >> >> I hadn't thought about checking the version, but I really should. The >> current code will only work with 3.0.3 but less than 3.1.0 (not yet >> released at last check). >> >> >reason that you're bothering to explicitly wrap the import? >> >> Yes, it was suggested by Krister. > > Okay (I skipped most of that subthread, since I was away). It's not clear > to me that this is useful -- on a system that hasn't been hacked to bits, > it'll be there (otherwise SUNWipkg won't even install). I'll want this to > go away, though, certainly once cherrypy gets properly integrated. I have no objections to removing the check completely if so desired. Just let me know. >> > - on line 17, you return a status code, but on line 191, you raise an >> >exception. What's the difference? >> >> ilne 17? I'm not sure what line you're referring to. However, I tried >> to keep most of this code the same it was before. So if it is >> returning one now that's probably why. Can you clarify? > > Errr. Not sure about my original intended line number, but line 79 will > do. Basically, I can't tell why in some cases you're raising exceptions to > make the server return a particular HTTP status code and why in some cases > you're doing so via a return value. Is there a useful difference between > the two methods? If not, shouldn't they all be done the same way? This was in transaction.py. In the case of transaction.py, the open() function is returning a particular status to the calling function. The calling function is explicitly evaluating the return value and then performing exceptions or various other bits of logic based on that return value. As such, it wasn't appropriate for me to raise an exception there. As for abandon, it was originally sending a response directly to the client, so I migrated it to do the same in CherryPy nomenclature, since the calling code is expecting abandon to handle it. So, in short, it's just doing what it used to. My personal preference is to let the caller handle abandon() and return the status since that would make it match open()'s behaviour. Would that be your desired approach? >> > - Do you think it would be feasible / cleaner to decorate each of the >> >operations methods with attributes designating what operation and >> >version they represent, and then build the vop table out of those >> >decorations, rather than the method names? My lack of understanding of >> >what's going on in default() might preclude that. >> >> It sounds like it would be possible. However, a pointer to the actual >> method is still needed for CherryPy's dispatch table. >> >> You can find more information about how its default dispatcher works here: >> http://www.cherrypy.org/wiki/CherryPyTutorial#Findingthecorrectobject >> >> and here: >> http://www.cherrypy.org/wiki/CherryPyTutorial#Partialmatchesandthedefaultmethod >> >> Of course a custom dispatcher could be written which would certainly >> change things. For simplicity's sake, I was trying to stick with the >> default one provided. > > Okay, I'll take a look, thanks. I'm certainly not going to hold your wad > back for this sort of exploration. I would be more than willing to revisit the dispatcher issue as a later enhancement, as I'm not terribly thrilled with the dummy object either. >> > SUNWpython-cherrypy/Makefile: >> > >> > - I wonder if perhaps doing the tarball download higher up, in the main >> >"make install" wouldn't be better -- so that it'll work out of the box, >> >and you could integrate it into "make link" and so on. >> >> Eventually cherrypy will go away and not be part of the build, so I'm >> not certain. > > Yeah, but we don't have a timeframe on that yet, do we? Seems like we > could be in the current situation for a while. True. So what are your expectations then in regards to this wad? >> > - line 34, 35: This needs to point to us/Sun, since we're the distributor >> >(vendor) here. >> >> Ah. I changed the vendor, what about the email? I've removed it for now. > > Email probably should be whatever it is for SUNWipkg. No EMAIL is specified, so I'll leave it out. >> I have posted an updated webrev here: >> http://cr.opensolaris.org/~swalker/pkg-depot-9/ >> >> ...and a udiff of the changes from the last webrev here: >> http://cr.opensolaris.org/~swalker/pkg-depot-9/raw_files/pkg-depot-9.patch > > Thanks. I'll look over them now. Thanks for the re-review. I'll get to these changes later tonight (I hope). Cheers, -- Shawn Walker ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/6/2 Danek Duvall <[EMAIL PROTECTED]>: > On Mon, Jun 02, 2008 at 11:39:54AM -0700, [EMAIL PROTECTED] wrote: > >> This would seem to violate the principle of least surprise. If we're >> able to determine that a component that isn't part of the default runtime >> is missing, it's better to emit a useful message instead of a traceback. > > Fair enough. We don't have any other such dependencies, do we? Not that I know of. -- Shawn Walker ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Mon, Jun 02, 2008 at 11:39:54AM -0700, [EMAIL PROTECTED] wrote: > This would seem to violate the principle of least surprise. If we're > able to determine that a component that isn't part of the default runtime > is missing, it's better to emit a useful message instead of a traceback. Fair enough. We don't have any other such dependencies, do we? Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
> > >reason that you're bothering to explicitly wrap the import? > > Okay (I skipped most of that subthread, since I was away). It's not clear > to me that this is useful -- on a system that hasn't been hacked to bits, > it'll be there (otherwise SUNWipkg won't even install). I'll want this to > go away, though, certainly once cherrypy gets properly integrated. This would seem to violate the principle of least surprise. If we're able to determine that a component that isn't part of the default runtime is missing, it's better to emit a useful message instead of a traceback. When Shawn and I had this converstion, I'm not sure we had finalized how CherryPy was being delivered. My concern was that users/developers would get the depot independently of CherryPy and that there would be the potential for missing components and version mismatches. -j ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Sun, Jun 01, 2008 at 10:27:13PM -0500, Shawn Walker wrote: > http://cr.opensolaris.org/~swalker/pkg-depot-9/ catalog.py: - line 1167: why the return statement here? repository.py: - line 90: you'll end up with two adjacent spaces here. - line 101: capitalize first word of sentence. > > - line 141: I'm confused on how we might get here. It seems like no > >matter what you do, unless you get shunted into the face code above, > >we're going to error out of this method. > > I think you're right; it looks like a leftover from the old logic. I'm still confused here. If we don't go into the face code, does this method do anything useful other than return errors? What happens if line 142 doesn't fire? Is there any point in the code in this method from line 131 onwards? pkginfo: - line 28: Huh? I suppose you could simply say "all" here, but we typically don't tag packages as being architecture neutral, even if they are. > > SUNWpython-cherrypy/Makefile: > > > > - If nothing else, your "prototype" target rule should be broken up into > >more rules. > > I'm not sure what you wanted, but I have split it up and simplified it > some more. That's better. I'd probably go one step further and put the cherrypy build in its own target as well. Right now, it's going to rebuild cherrypy every single time you type make install. I'm not too annoyed by this yet, though, so maybe I'll be nice and fix it myself sometime later. Thanks, Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Sun, Jun 01, 2008 at 10:27:13PM -0500, Shawn Walker wrote: > > - line 72: you're not actually doing a version check here. Is there a > > I hadn't thought about checking the version, but I really should. The > current code will only work with 3.0.3 but less than 3.1.0 (not yet > released at last check). > > >reason that you're bothering to explicitly wrap the import? > > Yes, it was suggested by Krister. Okay (I skipped most of that subthread, since I was away). It's not clear to me that this is useful -- on a system that hasn't been hacked to bits, it'll be there (otherwise SUNWipkg won't even install). I'll want this to go away, though, certainly once cherrypy gets properly integrated. > > - on line 17, you return a status code, but on line 191, you raise an > >exception. What's the difference? > > ilne 17? I'm not sure what line you're referring to. However, I tried > to keep most of this code the same it was before. So if it is > returning one now that's probably why. Can you clarify? Errr. Not sure about my original intended line number, but line 79 will do. Basically, I can't tell why in some cases you're raising exceptions to make the server return a particular HTTP status code and why in some cases you're doing so via a return value. Is there a useful difference between the two methods? If not, shouldn't they all be done the same way? > > - Do you think it would be feasible / cleaner to decorate each of the > >operations methods with attributes designating what operation and > >version they represent, and then build the vop table out of those > >decorations, rather than the method names? My lack of understanding of > >what's going on in default() might preclude that. > > It sounds like it would be possible. However, a pointer to the actual > method is still needed for CherryPy's dispatch table. > > You can find more information about how its default dispatcher works here: > http://www.cherrypy.org/wiki/CherryPyTutorial#Findingthecorrectobject > > and here: > http://www.cherrypy.org/wiki/CherryPyTutorial#Partialmatchesandthedefaultmethod > > Of course a custom dispatcher could be written which would certainly > change things. For simplicity's sake, I was trying to stick with the > default one provided. Okay, I'll take a look, thanks. I'm certainly not going to hold your wad back for this sort of exploration. > > SUNWpython-cherrypy/Makefile: > > > > - I wonder if perhaps doing the tarball download higher up, in the main > >"make install" wouldn't be better -- so that it'll work out of the box, > >and you could integrate it into "make link" and so on. > > Eventually cherrypy will go away and not be part of the build, so I'm > not certain. Yeah, but we don't have a timeframe on that yet, do we? Seems like we could be in the current situation for a while. > > - Why is the CDDL here? > > I was copying the approach from the other files. > > I have removed it and the Sun copyright notice. Please correct me if I'm > wrong. That should be right -- the license and copyright are for the bits delivered by the package, not for the copyright file itself. > > - line 34, 35: This needs to point to us/Sun, since we're the distributor > >(vendor) here. > > Ah. I changed the vendor, what about the email? I've removed it for now. Email probably should be whatever it is for SUNWipkg. > I have posted an updated webrev here: > http://cr.opensolaris.org/~swalker/pkg-depot-9/ > > ...and a udiff of the changes from the last webrev here: > http://cr.opensolaris.org/~swalker/pkg-depot-9/raw_files/pkg-depot-9.patch Thanks. I'll look over them now. Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/30 Danek Duvall <[EMAIL PROTECTED]>: >> http://cr.opensolaris.org/~swalker/pkg-depot-8/ > > src/depot.py: > > - line 45: I think this should be "opensolaris.org". The auth name isn't >the FQDN of the server, and we should be more biased towards opensolaris >than towards sun.com. That was the original value. However, I agree and have changed it. > - line 55, 59: are these things that we'd want to be able to tune? THREADS_MIN and THREADS_MAX are not expected to be tuned. They are only used as a way to ensure that someone doesn't start the depot server with a bad number of threads. CherryPy does not currently support dynamic adjustment of the number of threads, meaning it starts all the threads specified at server startup. However, I could understand the desire to change SOCKET_TIMEOUT_DEFAULT. I have added a new option (-t) and updated the documentation and smf manifest to reflect this. > - line 72: you're not actually doing a version check here. Is there a I hadn't thought about checking the version, but I really should. The current code will only work with 3.0.3 but less than 3.1.0 (not yet released at last check). >reason that you're bothering to explicitly wrap the import? Yes, it was suggested by Krister. > - line 113: why not use sets? Existence in the set is the equivalent of >the get returning True. Changed. > - line 137: I think I'd rather just see three except clauses, even if you >have to call usage() in three separate places. I admit, it would be cleaner. Changed. > - line 147: do this as a try/except block Changed. > - line 164: I think this is an exit(1). 2 should be for option >processing errors, as per the man page. I didn't even notice that on the man page. Thanks, changed. > - line 176: use double-quotes here, and no space before colons in dict >literals. Plus we've been using four-space indents here. Changed. > - line 198: space around equals. Changed. > catalog.py: > > - Why not just define outfile() in terms of output() -- just write >everything you get from output()? Same in updatelog.py. Changed. > server/__init__.py: > > - Might as well torch the SCCS ident line here. Changed. > server/depot.py: > > - It would be really useful to have a block comment explaining what this >file is for -- why it was grabbed from the cherrypy code, and what >changes you've made to it. I'm not sure if the comment at line 74 is >that comment, but if so, it should probably be moved up to the top and >made more obvious that it's not part of the original code. The comment at line 74 is basically the reason, however, I have added a clarifying docstring to the new class. > server/face.py: > > - line 154, 159: don't need ".keys()" here -- the "in" operator works on >the dictionary itself as you'd expect. Changed. > - line 163: doesn't this call need three arguments? Oops. Fixed. > server/transaction.py: > > - line 176, 177: spaces around equals Fixed. > - on line 17, you return a status code, but on line 191, you raise an >exception. What's the difference? ilne 17? I'm not sure what line you're referring to. However, I tried to keep most of this code the same it was before. So if it is returning one now that's probably why. Can you clarify? > updatelog.py: > > - line 359: as long as you're doing other cleanup here, might as well >change "it" to "is" here. Fixed. > server/repository.py: > > - line 78: no need for parens around return values Changed. > - line 89: continuation indents are four spaces Fixed. > - line 98: maybe a short comment on why we need an object, rather than an >empty string, or None, or some other core Python object? Added. > - line 141: I'm confused on how we might get here. It seems like no >matter what you do, unless you get shunted into the face code above, >we're going to error out of this method. I think you're right; it looks like a leftover from the old logic. > - line 294: Why not just catch socket.error directly? True, that was rather silly of me. Fixed. > - I assume there's no decorator to tell cherrypy not to decode URL >components? Not that I know of. If you want the raw request path, you can get it from request.path_info or from the environ. > - Do you think it would be feasible / cleaner to decorate each of the >operations methods with attributes designating what operation and >version they represent, and then build the vop table out of those >decorations, rather than the method names? My lack of understanding of >what's going on in default() might preclude that. It sounds like it would be possible. However, a pointer to the actual method is still needed for CherryPy's dispatch table. You can find more information about how its default dispatcher works here: http://www.cherrypy.org/wiki/CherryPyTutorial#Findingthecorrectobject and here: http://www.cherrypy.org/wiki/CherryPyTutorial#Partialmatches
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
> http://cr.opensolaris.org/~swalker/pkg-depot-8/ src/depot.py: - line 45: I think this should be "opensolaris.org". The auth name isn't the FQDN of the server, and we should be more biased towards opensolaris than towards sun.com. - line 55, 59: are these things that we'd want to be able to tune? - line 72: you're not actually doing a version check here. Is there a reason that you're bothering to explicitly wrap the import? - line 113: why not use sets? Existence in the set is the equivalent of the get returning True. - line 137: I think I'd rather just see three except clauses, even if you have to call usage() in three separate places. - line 147: do this as a try/except block - line 164: I think this is an exit(1). 2 should be for option processing errors, as per the man page. - line 176: use double-quotes here, and no space before colons in dict literals. Plus we've been using four-space indents here. - line 198: space around equals. catalog.py: - Why not just define outfile() in terms of output() -- just write everything you get from output()? Same in updatelog.py. server/__init__.py: - Might as well torch the SCCS ident line here. server/depot.py: - It would be really useful to have a block comment explaining what this file is for -- why it was grabbed from the cherrypy code, and what changes you've made to it. I'm not sure if the comment at line 74 is that comment, but if so, it should probably be moved up to the top and made more obvious that it's not part of the original code. server/face.py: - line 154, 159: don't need ".keys()" here -- the "in" operator works on the dictionary itself as you'd expect. - line 163: doesn't this call need three arguments? server/transaction.py: - line 176, 177: spaces around equals - on line 17, you return a status code, but on line 191, you raise an exception. What's the difference? updatelog.py: - line 359: as long as you're doing other cleanup here, might as well change "it" to "is" here. server/repository.py: - line 78: no need for parens around return values - line 89: continuation indents are four spaces - line 98: maybe a short comment on why we need an object, rather than an empty string, or None, or some other core Python object? - line 141: I'm confused on how we might get here. It seems like no matter what you do, unless you get shunted into the face code above, we're going to error out of this method. - line 294: Why not just catch socket.error directly? - I assume there's no decorator to tell cherrypy not to decode URL components? - Do you think it would be feasible / cleaner to decorate each of the operations methods with attributes designating what operation and version they represent, and then build the vop table out of those decorations, rather than the method names? My lack of understanding of what's going on in default() might preclude that. SUNWpython-cherrypy/Makefile: - I wonder if perhaps doing the tarball download higher up, in the main "make install" wouldn't be better -- so that it'll work out of the box, and you could integrate it into "make link" and so on. - If nothing else, your "prototype" target rule should be broken up into more rules. - line 41: you're missing an "IPS" here. SUNWpython-cherrypy/copyright: - Why is the CDDL here? SUNWpython-cherrypy/pkginfo: - line 25: no trailing "thon" in the package name. - line 30: This needs to always be "1.0". - line 34, 35: This needs to point to us/Sun, since we're the distributor (vendor) here. Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Hi Shawn, Sorry I'm a bit too busy to review your changes at the moment, but I did want to tell you that I think I own a bug 1?? which is related to the SMF property names, so you may want to check that out too. Trev Shawn Walker wrote: 2008/5/21 Shawn Walker <[EMAIL PROTECTED]>: 2008/5/20 Shawn Walker <[EMAIL PROTECTED]>: 2008/5/11 Shawn Walker <[EMAIL PROTECTED]>: The following webrev includes proposed fixes for the following bugs: 1854 rework depot to use higher-level framework 1154 pkg.depotd tracebacks when given bad options 1237 pkg.depotd -p 80 -p 90 should return usage, but second option is taken 1887 depot status page output is not valid HTML 1888 memleaks test overwrites PYTHONPATH Newly added: 1956 depotd performance hampered by unnecessary getpwuid/getgrgid calls Newly added: 1889 pkg.depotd(1M) man page specifies incorrect smf(5) property names 2021 depotd --rebuild doesn't work if specified before -d Reasons for new webrev: * I realised after the last webrev that I had forgotten to update the usage message and man pages for the new -s option I added so you could control how many threads to serve requests were started. As such, it made sense to fix 1889 while I did that. * I happened to see bug 2021 and realised that my command-line option parsing fixes would not be complete without fixing this as well. Changes since last webrev: * Updated man page to document the previously added -s option. * Updated SMF manifest to add pkg/threads property for -s option. * Added CherryPy's *.tar.gz to hgignore. * Moved rebuild/readonly and repository configuration logic to happen after all command-line options have been successfully parsed. * Moved and simplified comments for socket timeout. * Simplified quickstart() call to start server. To make it easier to see the changes since the last webrev, I have created a special diff: http://cr.opensolaris.org/~swalker/pkg-depot-8/raw_files/pkg-depot-8.patch New webrev: http://cr.opensolaris.org/~swalker/pkg-depot-8/ I apologise for adding yet another set of changes to this already long review. I should have thought of these earlier. Cheers, smime.p7s Description: S/MIME Cryptographic Signature ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/21 Shawn Walker <[EMAIL PROTECTED]>: > 2008/5/20 Shawn Walker <[EMAIL PROTECTED]>: >> 2008/5/11 Shawn Walker <[EMAIL PROTECTED]>: >>> The following webrev includes proposed fixes for the following bugs: >>> >>>1854 rework depot to use higher-level framework >>>1154 pkg.depotd tracebacks when given bad options >>>1237 pkg.depotd -p 80 -p 90 should return usage, but second option is >>> taken >>>1887 depot status page output is not valid HTML >>>1888 memleaks test overwrites PYTHONPATH >> >> Newly added: >> 1956 depotd performance hampered by unnecessary getpwuid/getgrgid calls Newly added: 1889 pkg.depotd(1M) man page specifies incorrect smf(5) property names 2021 depotd --rebuild doesn't work if specified before -d Reasons for new webrev: * I realised after the last webrev that I had forgotten to update the usage message and man pages for the new -s option I added so you could control how many threads to serve requests were started. As such, it made sense to fix 1889 while I did that. * I happened to see bug 2021 and realised that my command-line option parsing fixes would not be complete without fixing this as well. Changes since last webrev: * Updated man page to document the previously added -s option. * Updated SMF manifest to add pkg/threads property for -s option. * Added CherryPy's *.tar.gz to hgignore. * Moved rebuild/readonly and repository configuration logic to happen after all command-line options have been successfully parsed. * Moved and simplified comments for socket timeout. * Simplified quickstart() call to start server. To make it easier to see the changes since the last webrev, I have created a special diff: http://cr.opensolaris.org/~swalker/pkg-depot-8/raw_files/pkg-depot-8.patch New webrev: http://cr.opensolaris.org/~swalker/pkg-depot-8/ I apologise for adding yet another set of changes to this already long review. I should have thought of these earlier. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Shawn, Please see the other thread I started about distutils vs. make. If the resolution is to just stick with make, then there won't be any need to do anything with setup.py. My hope was that one could do the following (when cherrypy isn't on your system): hg clone ... ips cd ips\src python setup.py install python setup.py test and everything would build and the unit tests would run, including whatever is necessary with cherrypy to make this work. Tom Shawn Walker wrote: 2008/5/22 Tom Mueller (pkg-discuss) <[EMAIL PROTECTED]>: My request was for setup.py to be able to obtain the cherrypy distribution like the Makefile (if the tar.gz file is not going to be checked into hg), not to build an SVR4 package for cherrypy. I misunderstood then. So is it acceptable to have setup.py grab the CherryPy tarball if it doesn't exist? That still wouldn't build it and install it into the proto area. Cheers, begin:vcard fn:Tom Mueller n:Mueller;Tom org:Sun Microsystems, Inc.;Update Center/OpenInstaller Software adr:;;21915 Hillandale Dr;Elkhorn;NE;68022;USA email;internet:[EMAIL PROTECTED] title:Senior Staff Engineer tel;work:877-250-4011 tel;fax:877-250-4011 tel;home:402-916-9943 x-mozilla-html:TRUE version:2.1 end:vcard ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/22 Tom Mueller (pkg-discuss) <[EMAIL PROTECTED]>: > My request was for setup.py to be able to obtain the cherrypy distribution > like the Makefile (if the tar.gz file is not going to be checked into hg), > not to build an SVR4 package for cherrypy. I misunderstood then. So is it acceptable to have setup.py grab the CherryPy tarball if it doesn't exist? That still wouldn't build it and install it into the proto area. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
My request was for setup.py to be able to obtain the cherrypy distribution like the Makefile (if the tar.gz file is not going to be checked into hg), not to build an SVR4 package for cherrypy. Tom Stephen Hahn wrote: * Shawn Walker <[EMAIL PROTECTED]> [2008-05-22 17:49]: 2008/5/22 <[EMAIL PROTECTED]>: Hi Shawn, To make it easier to see the changes since the last webrev, I have created a special diff: http://cr.opensolaris.org/~swalker/pkg-depot-7/raw_files/pkg-depot-7.patch New webrev: http://cr.opensolaris.org/~swalker/pkg-depot-7/ I took a look at both of these. I think this looks good. I can't think of anything else that I think you need to do. Tom mentioned changing setup.py to do what I have the Makefile doing now for CherryPy. Hold on. CherryPy is with us temporarily, and will ultimately live in a different consolidation from pkg. If pkg moves to distutils (or setuputils), it wouldn't be right for its setup.py to be building CherryPy. Maybe we can make the Makefile invoke pkg's setup.py, and then use some other tool on Windows for invoking the two setup.py pieces? - Stephen ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/22 Stephen Hahn <[EMAIL PROTECTED]>: > * Shawn Walker <[EMAIL PROTECTED]> [2008-05-22 17:49]: >> 2008/5/22 <[EMAIL PROTECTED]>: >> > Hi Shawn, >> > >> >> To make it easier to see the changes since the last webrev, I have >> >> created a special diff: >> >> http://cr.opensolaris.org/~swalker/pkg-depot-7/raw_files/pkg-depot-7.patch >> >> >> >> New webrev: >> >> http://cr.opensolaris.org/~swalker/pkg-depot-7/ >> > >> > I took a look at both of these. I think this looks good. I can't think >> > of anything else that I think you need to do. >> >> Tom mentioned changing setup.py to do what I have the Makefile doing >> now for CherryPy. > > Hold on. CherryPy is with us temporarily, and will ultimately live in > a different consolidation from pkg. If pkg moves to distutils (or > setuputils), it wouldn't be right for its setup.py to be building > CherryPy. > > Maybe we can make the Makefile invoke pkg's setup.py, and then use some > other tool on Windows for invoking the two setup.py pieces? I have no particular preference, and I'm open to suggestions. Tom? -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
* Shawn Walker <[EMAIL PROTECTED]> [2008-05-22 17:49]: > 2008/5/22 <[EMAIL PROTECTED]>: > > Hi Shawn, > > > >> To make it easier to see the changes since the last webrev, I have > >> created a special diff: > >> http://cr.opensolaris.org/~swalker/pkg-depot-7/raw_files/pkg-depot-7.patch > >> > >> New webrev: > >> http://cr.opensolaris.org/~swalker/pkg-depot-7/ > > > > I took a look at both of these. I think this looks good. I can't think > > of anything else that I think you need to do. > > Tom mentioned changing setup.py to do what I have the Makefile doing > now for CherryPy. Hold on. CherryPy is with us temporarily, and will ultimately live in a different consolidation from pkg. If pkg moves to distutils (or setuputils), it wouldn't be right for its setup.py to be building CherryPy. Maybe we can make the Makefile invoke pkg's setup.py, and then use some other tool on Windows for invoking the two setup.py pieces? - Stephen -- [EMAIL PROTECTED] http://blogs.sun.com/sch/ ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/22 <[EMAIL PROTECTED]>: > Hi Shawn, > >> To make it easier to see the changes since the last webrev, I have >> created a special diff: >> http://cr.opensolaris.org/~swalker/pkg-depot-7/raw_files/pkg-depot-7.patch >> >> New webrev: >> http://cr.opensolaris.org/~swalker/pkg-depot-7/ > > I took a look at both of these. I think this looks good. I can't think > of anything else that I think you need to do. Tom mentioned changing setup.py to do what I have the Makefile doing now for CherryPy. I'll try do to that tonight. Also, I won't be doing a putuback on this until Danek has had a chance to review it, at his request. > Out of curiosity, how did you find the corner case that necessitated the > addition of the on_end_request hook? I've been accused of purposefully trying to break programs by many people over the years :-) Actually, it was because of the work I've been doing on bug 115 to change the client to "not have a cow on sigpipe." As a result, I've been looking for broken pipe errors. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Hi Shawn, > To make it easier to see the changes since the last webrev, I have > created a special diff: > http://cr.opensolaris.org/~swalker/pkg-depot-7/raw_files/pkg-depot-7.patch > > New webrev: > http://cr.opensolaris.org/~swalker/pkg-depot-7/ I took a look at both of these. I think this looks good. I can't think of anything else that I think you need to do. Out of curiosity, how did you find the corner case that necessitated the addition of the on_end_request hook? Thanks, -j ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/20 Shawn Walker <[EMAIL PROTECTED]>: > 2008/5/11 Shawn Walker <[EMAIL PROTECTED]>: >> The following webrev includes proposed fixes for the following bugs: >> >>1854 rework depot to use higher-level framework >>1154 pkg.depotd tracebacks when given bad options >>1237 pkg.depotd -p 80 -p 90 should return usage, but second option is >> taken >>1887 depot status page output is not valid HTML >>1888 memleaks test overwrites PYTHONPATH > > Newly added: > 1956 depotd performance hampered by unnecessary getpwuid/getgrgid calls Try two (sorry for the first repost): Changes since last webrev: * Per Stephen, Johansen, I have changed the SUNWpython-cherrypy makefile to: -- Check to see if CherryPy-3.0.3.tar.gz exists in the package directory -- If it doesn't, it uses wget to retrieve it -- If it can't retrieve it, it will error and tell you what file you need to get and where to put it -- If it does exist, it decompresses it and installs it into proto * Per Johansen, I have added a -s parameter to depot.py so you can specify the number of threads for the cherrypy server thread pool. It also enforces a minimum of 1 and maximum of 100 for this parameter. * I have changed all the "hardcoded" values in depot.py for port, etc. into "constants" and placed them at the top of the file * I have fixed a regression in the command-line parsing to not have a traceback when an unknown option is specified * I have added an extensive comment explaining why the cherrypy.response.write change was needed in DepotResponse in depot.py * I have simplified repository.py's code for versions_0, filelist_0 and added lots of comments due to the "magic" involved in the latter * I added some special exception handling so that if the client is interrupted (broken pipe) the server won't throw a traceback or exception To make it easier to see the changes since the last webrev, I have created a special diff: http://cr.opensolaris.org/~swalker/pkg-depot-7/raw_files/pkg-depot-7.patch New webrev: http://cr.opensolaris.org/~swalker/pkg-depot-7/ Notes: To answer an earlier question, "cherrypy.request and cherrypy.response are thread-local objects, so that they are always accessible as globals (yet without the risk of concurrency issues)." Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/20 Shawn Walker <[EMAIL PROTECTED]>: > 2008/5/11 Shawn Walker <[EMAIL PROTECTED]>: >> The following webrev includes proposed fixes for the following bugs: >> >>1854 rework depot to use higher-level framework >>1154 pkg.depotd tracebacks when given bad options >>1237 pkg.depotd -p 80 -p 90 should return usage, but second option is >> taken >>1887 depot status page output is not valid HTML >>1888 memleaks test overwrites PYTHONPATH > > Newly added: > 1956 depotd performance hampered by unnecessary getpwuid/getgrgid calls > > I believe I have now resolved the previous heap size issues with > streaming large files. > > I had the opportunity this evening to discuss the streaming issue with > one of the lead cherrypy developers. Initially, he also suggested > using wsgi middleware just as the other person had. However, after > further discussion, he came up with an alternate solution that works > by subclassing and overriding CherryPy's response class object. This > solution allows us to write directly to the client (as we wanted all > along) just as we hada previously with BaseHTTPServer request.rfile. > > After performing a "pkg install ss-dev" into an empty user image, pmap > reported this: > ... > 080610005360K rwx--[ heap ] > ... > total 14904K > > Repeats of the above oepration never increased the heap size more than > 40KiB; there were also times it did not increase at all. Please verify > this when reviewing. > > webrev: > http://cr.opensolaris.org/~swalker/pkg-depot-6/ > > HG: changed src/Makefile > HG: changed src/depot.py > HG: changed src/modules/server/__init__.py > HG: changed src/modules/server/depot.py > HG: changed src/modules/server/repository.py > > Notes: > * tarball I added *last* time: > src/pkgdefs/SUNWpython-cherrypy/CherryPy-3.0.3.tar.gz > * You must start the depot with an explicit port number argument (e.g. > -p 1) otherwise it will try to connect to port 80 > > Cheers, > -- > Shawn Walker > > "To err is human -- and to blame it on a computer is even more so." - > Robert Orben > -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/11 Shawn Walker <[EMAIL PROTECTED]>: > The following webrev includes proposed fixes for the following bugs: > >1854 rework depot to use higher-level framework >1154 pkg.depotd tracebacks when given bad options >1237 pkg.depotd -p 80 -p 90 should return usage, but second option is > taken >1887 depot status page output is not valid HTML >1888 memleaks test overwrites PYTHONPATH Newly added: 1956 depotd performance hampered by unnecessary getpwuid/getgrgid calls I believe I have now resolved the previous heap size issues with streaming large files. I had the opportunity this evening to discuss the streaming issue with one of the lead cherrypy developers. Initially, he also suggested using wsgi middleware just as the other person had. However, after further discussion, he came up with an alternate solution that works by subclassing and overriding CherryPy's response class object. This solution allows us to write directly to the client (as we wanted all along) just as we hada previously with BaseHTTPServer request.rfile. After performing a "pkg install ss-dev" into an empty user image, pmap reported this: ... 080610005360K rwx--[ heap ] ... total 14904K Repeats of the above oepration never increased the heap size more than 40KiB; there were also times it did not increase at all. Please verify this when reviewing. webrev: http://cr.opensolaris.org/~swalker/pkg-depot-6/ HG: changed src/Makefile HG: changed src/depot.py HG: changed src/modules/server/__init__.py HG: changed src/modules/server/depot.py HG: changed src/modules/server/repository.py Notes: * tarball I added *last* time: src/pkgdefs/SUNWpython-cherrypy/CherryPy-3.0.3.tar.gz * You must start the depot with an explicit port number argument (e.g. -p 1) otherwise it will try to connect to port 80 Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/19 <[EMAIL PROTECTED]>: > Shawn, > >> So I reworked the filelist operation again to use StringIO initially, >> and it worked this time. Going further, I discovered I could get a >> little better performance using cStringIO instead. >> >> As such, I believe I've managed to find an acceptable solution with >> cStringIO. After each file is written to the tar stream, I stream the >> cStringIO contents and then truncate and add the next file to the tar >> stream. > > This is an improvement. Would you comment the code you added in > repository.py. I had to take a look at the source for cStringIO to > figure out some of the details. Sure. I actually never looked at the source for cStringIO; I just used the pydoc. >> The memory footprint is going to be a little bigger than what we >> currently have; though acceptably so in my view. > > Let's validate this empirically. The cStringIO code allocates from the > heap. Even when we free a buffer, the memory will stay allocated in > Python's address space. This means that over time, the memory allocated > by Python may gradually grow. I'm going to run a test where we pull a > bunch of large files from the depot. This should show us how the change > effects our overall memory footprint. Argh! I had also checked pmap's output yesterday, but apparently I had checked the *wrong* depot process. The final pmap output today (after rechecking) shows about 43MiB at the end of a large operation with a heap of about 34MiB. It doesn't grow any larger on subsequent operations, but it looks like you were right to have me check again. Sorry about that. > If this turns out to be a large number, we may want to consider writing > a custom cStringIO-like object that has a hard limit on the buffer size. > This may be a bit tricky, though. The Python code in tarfile.py assumes > that all operations can complete synchronously. If we run out of space > and block, the op will never complete. Yes, I saw the note about that. I'll have to experiment some with a wrapper object. >> By comparison, the old depot code allocated and freed a total of about >> 1MiB *every* time the operation is performed since it starts and kills >> a thread for every transaction. >> >> I obtained that information using the anonprofile.d DTrace script that >> Brendan Gregg wrote. > > cStringIO allocates from the heap. Does anonprofile track those > allocations, or just mmap(MAP_ANON) ones? anonprofile.d actually hooks into the kernel's anon_resvmem/anon_unresvmem functions. http://www.brendangregg.com/DTrace/anonprofile.d >> > I'd be interested to see the example that the cherrypy guys gave you, if >> > it's handy. >> >> This is the example they pointed me to: >> http://www.cherrypy.org/browser/trunk/cherrypy/test/test_conn.py?rev=1956#L282 > > I took a look at this example. Unless I misread the code, it looks like > they're keeping the connection open and sending a request, reading a > response, and then sending another request. This doesn't fit my > definition of pipelining, since we want to send multiple requests at > once and then receive the responses. I thought you might say that :-) When I get the time, I was going to try out their approach, except using the pattern you wanted. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Shawn, > So I reworked the filelist operation again to use StringIO initially, > and it worked this time. Going further, I discovered I could get a > little better performance using cStringIO instead. > > As such, I believe I've managed to find an acceptable solution with > cStringIO. After each file is written to the tar stream, I stream the > cStringIO contents and then truncate and add the next file to the tar > stream. This is an improvement. Would you comment the code you added in repository.py. I had to take a look at the source for cStringIO to figure out some of the details. > The memory footprint is going to be a little bigger than what we > currently have; though acceptably so in my view. Let's validate this empirically. The cStringIO code allocates from the heap. Even when we free a buffer, the memory will stay allocated in Python's address space. This means that over time, the memory allocated by Python may gradually grow. I'm going to run a test where we pull a bunch of large files from the depot. This should show us how the change effects our overall memory footprint. If this turns out to be a large number, we may want to consider writing a custom cStringIO-like object that has a hard limit on the buffer size. This may be a bit tricky, though. The Python code in tarfile.py assumes that all operations can complete synchronously. If we run out of space and block, the op will never complete. > By comparison, the old depot code allocated and freed a total of about > 1MiB *every* time the operation is performed since it starts and kills > a thread for every transaction. > > I obtained that information using the anonprofile.d DTrace script that > Brendan Gregg wrote. cStringIO allocates from the heap. Does anonprofile track those allocations, or just mmap(MAP_ANON) ones? > > I'd be interested to see the example that the cherrypy guys gave you, if > > it's handy. > > This is the example they pointed me to: > http://www.cherrypy.org/browser/trunk/cherrypy/test/test_conn.py?rev=1956#L282 I took a look at this example. Unless I misread the code, it looks like they're keeping the connection open and sending a request, reading a response, and then sending another request. This doesn't fit my definition of pipelining, since we want to send multiple requests at once and then receive the responses. -j ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/11 Shawn Walker <[EMAIL PROTECTED]>: > The following webrev includes proposed fixes for the following bugs: > >1854 rework depot to use higher-level framework >1154 pkg.depotd tracebacks when given bad options >1237 pkg.depotd -p 80 -p 90 should return usage, but second option is > taken >1887 depot status page output is not valid HTML >1888 memleaks test overwrites PYTHONPATH An updated webrev is posted here: http://cr.opensolaris.org/~swalker/pkg-depot-5/ Change Summary * Per Tom, Unused usage() function in modules/server/repository.py was removed * I removed an unused import for sys from modules/server/repository.py * Per Stephen, a package was added for cherrypy and it was made part of the "make install" process such that it will install cherrypy into the proto area. Care was taken to ensure that the SUNWpython-cherrypy package remains separate from SUNWipkg. * I added the repository module to the modules/server/__init__.py * Per Johansen, I added an assert check for cherrypy import to depot.py. * Per Johansen, I removed all usage of the cherrypy singleton (and the import) from any client modules * Per Johansen, I changed the repository __init__ to not just skip un-exposed functions and instead assert if find a matching one during setup * Per Johansen, I reworked filelist to stream the tar content as we generate it instead of writing it to a temporary file and then streaming that Additional Notes * Performance testing so far shows that: -- New depot code is about 20% faster for a distro import (Danek reported this) -- Thread performance is far better as cherrypy starts about 10 lwps at startup and then just idles them as needed. A "pkg install SUNWfirefox" into an empty user image produced no thread creations / destructions. By comparison, the old depot code created and destroyed 1,073. -- Install time, at this point, on the client-side seems to be the same as it was before. -- The new depot code has caused about a 1-2MiB increase in process size for the depot server when idle. It allocates and deallocates up to 30MiB during example install operations the first time around, but this drops off to about 500KiB after the first time the operation is performed. By comparison, the old depot code free and allocated about 1MiB every request. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/16 <[EMAIL PROTECTED]>: >> I did post a question to the cherrypy users group though, so I'll see >> if someone knows a better way. > > Okay, great. Let me know if you'd like me to particiapte in the > discussion on their list. > > I spent some time this afternoon looking at the CherryPy code and some > of the Tools() that they've implemented. The code that handles the > gzipping of a stream seems awfully close to what we want to do, except > that we actually want to bypass the stack of transforms they employ in > the before_finalizer hooks. > > We may end up having to write some kind of file-like object that holds > the lines it gets from tarfile and then writes them out to the response > body. I'd be interested to see if the cherrypy users group knows of a > good way to do this. > > The response object has a attribute named stream (or streaming, maybe?) > and if that gets set, the framework omits key content-length headers > that could cause mischief. I did post to the list and the response I got was that what I was doing now was really the only way. As for file-like object that holds the lines, I had already experimented with that using StringIO. However, due to an idiotic mistake on my part, it never worked quite right. So I reworked the filelist operation again to use StringIO initially, and it worked this time. Going further, I discovered I could get a little better performance using cStringIO instead. As such, I believe I've managed to find an acceptable solution with cStringIO. After each file is written to the tar stream, I stream the cStringIO contents and then truncate and add the next file to the tar stream. This isn't as efficient as the old approach, but it does avoid creating a temporary file. >> I think, unfortunately, that this will be an issue I'll have to >> resolve somehow before this gets putback. > > We'll need to figure something out before putback. The streaming is > necessary for the production servers handling pkg.opensolaris.org. > It'll be a problem if their memory footprint suddenly baloons. I'll let > Stephen comment on this more, if it's appropriate. The memory footprint is going to be a little bigger than what we currently have; though acceptably so in my view. The increase in footprint, from what I can tell, is only about one to two megabytes for an idle depot process (meaning before and after an operation). During an operation (such as pkg install SUNWfirefox into an *empty* user image), anonymous memory profiling shows about a total of 30MiB being allocated and *freed* for the new depot code. That is only for the first time a client does this. Repeating the same operation (without stopping the depot server) shows only about 500KiB being freed and allocated. By comparison, the old depot code allocated and freed a total of about 1MiB *every* time the operation is performed since it starts and kills a thread for every transaction. I obtained that information using the anonprofile.d DTrace script that Brendan Gregg wrote. >> httplib2 can supposedly handle pipelined requests. >> >> The cherrypy guys also have an example of doing it "by hand" using the >> existing python libraries. > > I took a look at httplib2, but didn't see how it handled pipelined > requests. It looks like it employs keep-alive, so that you can send > multiple requests over the same socket; however, I didn't see any way to > send multiple requests at the same time. What did I miss? According to the authors, you should be able to perform multiple get requests and then receive the data. However, I have not found any examples. > I'd be interested to see the example that the cherrypy guys gave you, if > it's handy. This is the example they pointed me to: http://www.cherrypy.org/browser/trunk/cherrypy/test/test_conn.py?rev=1956#L282 >> >> One of the things I struggled with while making these changes was >> >> whether it was more efficient to pass the request and response object >> >> around (and cleaner) or whether it was better to simply use the >> >> singleton object to access them. >> > >> > My guess it that it might be faster to pass the request and response >> > object; however, the difference probably isn't enough to be appreciable. >> >> I'll take a look back at the code and see how big of a change it would >> be to do this. > > Unless you want to make this change, I wouldn't worry about it. I have further reduced the imports of cherrypy. Now only depot.py, server/repository.py and server/transaction.py use it. I'm going to write-up a small summary of the changes I've made since I first posted the depot code changes for review and then a link to the new webrev just to make it easier for others to look at. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/16 <[EMAIL PROTECTED]>: >> Thank you for reviewing this! > > You're welcome, of course. You're doing the hard part. :P > >> Notably, I am happy to report that the threading issues are definitely gone. > > That's awesome to see all of those lwp_create() calls dissapear. I'm > glad your change fixed this! > > Your performance numbers look good. I was curious about this one, > though: > >> install SUNWfirefox (old) >> -- >> real1m59.884s >> user0m17.175s >> sys 0m7.820s > >> install SUNWfirefox (new) >> -- >> real1m46.627s >> user0m32.920s >> sys 0m14.071s > > Your real time went down, implying that we didn't spend as much time > waiting around for the server. However, it looks like your user and sys > time increased. Do you have any explanation for your changes that > accounts for this? I wouldn't have expected the client to need to > perform more work. The sys time increase is probably due to the increased disk activity I'm seeing during processing. Remember that I'm serving the repository from, and installing a client image to, the same drive. That is most likely due to the current temporary file approach I'm using for filelist. As for the user time increase. I have no answer for that at the moment. I would have to do more profiling to give a better answer to that question, and I think I want to try to find a better solution for filelist first. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
> I did post a question to the cherrypy users group though, so I'll see > if someone knows a better way. Okay, great. Let me know if you'd like me to particiapte in the discussion on their list. I spent some time this afternoon looking at the CherryPy code and some of the Tools() that they've implemented. The code that handles the gzipping of a stream seems awfully close to what we want to do, except that we actually want to bypass the stack of transforms they employ in the before_finalizer hooks. We may end up having to write some kind of file-like object that holds the lines it gets from tarfile and then writes them out to the response body. I'd be interested to see if the cherrypy users group knows of a good way to do this. The response object has a attribute named stream (or streaming, maybe?) and if that gets set, the framework omits key content-length headers that could cause mischief. > I think, unfortunately, that this will be an issue I'll have to > resolve somehow before this gets putback. We'll need to figure something out before putback. The streaming is necessary for the production servers handling pkg.opensolaris.org. It'll be a problem if their memory footprint suddenly baloons. I'll let Stephen comment on this more, if it's appropriate. > httplib2 can supposedly handle pipelined requests. > > The cherrypy guys also have an example of doing it "by hand" using the > existing python libraries. I took a look at httplib2, but didn't see how it handled pipelined requests. It looks like it employs keep-alive, so that you can send multiple requests over the same socket; however, I didn't see any way to send multiple requests at the same time. What did I miss? I'd be interested to see the example that the cherrypy guys gave you, if it's handy. > >> One of the things I struggled with while making these changes was > >> whether it was more efficient to pass the request and response object > >> around (and cleaner) or whether it was better to simply use the > >> singleton object to access them. > > > > My guess it that it might be faster to pass the request and response > > object; however, the difference probably isn't enough to be appreciable. > > I'll take a look back at the code and see how big of a change it would > be to do this. Unless you want to make this change, I wouldn't worry about it. -j ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/16 <[EMAIL PROTECTED]>:
> Shawn,
>
>> > The approach that you're taking now doesn't need to employ streaming.
>> > You could just write the tarfile to a temporary file, serve it, and
>> > delete it. I'm not sure that's the right approach, though. Are you
>> > sure there's no sneaky way to get CherryPy to write a streaming response
>> > with an unknown length?
>>
>> The issue is not the unknown length; cherrypy doesn't care about that.
>>
>> The issue is that cherrypy does not provide a file object to write to
>> for the response.
>
> This was why I asked if there was some sleazy trick we could play to
> obtain the file object. The output has to go back to the requestor over
> a socket, so there's a descriptor for it _somewhere_. I just wondered
> if we might be able to get hold of it through the framework by doing
> things we weren't supposed to do.
I have not yet been able to find a way to do that.
Everything I've tried so far has resulted in "bad things".
I did post a question to the cherrypy users group though, so I'll see
if someone knows a better way.
>> One possible thought I had was to try creating a temporary file and
>> use that for the tarfile object, and after each tar_strem.add(),
>> stream the temporary file and then truncate it. I would repeat that
>> until done. However, I'm not sure if such chicanery would work :-)
>
> Me either. Another thought would be to write a separate streaming
> filelist daemon and use Apache's reverse proxy to map filelist ops to
> the separate service.
Yeah, the chicanery didn't work (one of the pkgsend tests failed, not
sure why yet). I tried using a temporary file, stringio, cStringIO,
etc.
I also tried mucking with:
cherrypy.request.rfile._sock.makefile('w')
cherrypy.request.rfile._sock._fileobject
I think, unfortunately, that this will be an issue I'll have to
resolve somehow before this gets putback.
Either that, or very soon after.
I know creating a temporary file the size of whatever we're streaming
isn't really practical when serving a large number of requests.
I just don't know of any other way to handle it at the moment.
>> > There has been a fair amount of hand-wringing about how filelist doesn't
>> > fit with our architectural principles. Perhaps now would be an
>> > opportune time to investigate whether we could switch to sitting behind
>> > Apache and simply pipeline our requests for multiple files?
>> > To be clear, I'm not implying that you should do that work all by
>> > yourself.
>>
>> I was actually considering that. The problem I have is that, as
>> Stephen pointed out, the current protocol must be supported for a
>> while. As such, that would have to be /filelist/1/.
>
> I was imagining that this would simply be a bunch of GET /file/0/
> requests pipelined together. We'd need to write a httplib that can
> issue pipelined requests, as no Python implementations seem to do this
> yet. I'm also not sure how many requests Apache will serve before it
> says enough is enough. (Can we request 64 different files at once?)
httplib2 can supposedly handle pipelined requests.
The cherrypy guys also have an example of doing it "by hand" using the
existing python libraries.
>> One of the things I struggled with while making these changes was
>> whether it was more efficient to pass the request and response object
>> around (and cleaner) or whether it was better to simply use the
>> singleton object to access them.
>
> My guess it that it might be faster to pass the request and response
> object; however, the difference probably isn't enough to be appreciable.
I'll take a look back at the code and see how big of a change it would
be to do this.
>> > repository.py:
>> > 307 - Does serve_file return a 404 if it can't find the file at the path
>> > that it has been given?
>>
>> If you specify a request path that cherrypy is unable to map to an
>> object through the "mounted object tree" (see quickstart in depot.py)
>> it will pass it off to the default page handler if you have one. If
>> you don't have one it will return a 404.
>>
>> So yes, currently the depot code is setup with a custom default page
>> handler that will return a 404 for unknown pages via face.py:unknown.
>
> I guess I may have asked this question obliquely. I was trying to
> figure out what happens if the client requests as filehash that isn't in
> the depot. The code in file_0 looks like this:
>
>return serve_file(os.path.normpath(os.path.join(
>self.scfg.file_root, misc.hash_file_name(fhash))),
>'application/data')
>
> So if fhash isn't in the depot and the path we pass to serve_file
> doesn't actually name a file, do we get a 404 here or an exception?
We get a 404. cherrypy handles that too :-)
> If I read your response correctly, this is a 404. I just wanted to make
> sure I understood.
It was actually a separate case, but the answer is the same.
In general, that was one of the things I liked the most: the fact that
che
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
> Thank you for reviewing this! You're welcome, of course. You're doing the hard part. :P > Notably, I am happy to report that the threading issues are definitely gone. That's awesome to see all of those lwp_create() calls dissapear. I'm glad your change fixed this! Your performance numbers look good. I was curious about this one, though: > install SUNWfirefox (old) > -- > real1m59.884s > user0m17.175s > sys 0m7.820s > install SUNWfirefox (new) > -- > real1m46.627s > user0m32.920s > sys 0m14.071s Your real time went down, implying that we didn't spend as much time waiting around for the server. However, it looks like your user and sys time increased. Do you have any explanation for your changes that accounts for this? I wouldn't have expected the client to need to perform more work. Thanks, -j ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Shawn, > > The approach that you're taking now doesn't need to employ streaming. > > You could just write the tarfile to a temporary file, serve it, and > > delete it. I'm not sure that's the right approach, though. Are you > > sure there's no sneaky way to get CherryPy to write a streaming response > > with an unknown length? > > The issue is not the unknown length; cherrypy doesn't care about that. > > The issue is that cherrypy does not provide a file object to write to > for the response. This was why I asked if there was some sleazy trick we could play to obtain the file object. The output has to go back to the requestor over a socket, so there's a descriptor for it _somewhere_. I just wondered if we might be able to get hold of it through the framework by doing things we weren't supposed to do. > One possible thought I had was to try creating a temporary file and > use that for the tarfile object, and after each tar_strem.add(), > stream the temporary file and then truncate it. I would repeat that > until done. However, I'm not sure if such chicanery would work :-) Me either. Another thought would be to write a separate streaming filelist daemon and use Apache's reverse proxy to map filelist ops to the separate service. > > There has been a fair amount of hand-wringing about how filelist doesn't > > fit with our architectural principles. Perhaps now would be an > > opportune time to investigate whether we could switch to sitting behind > > Apache and simply pipeline our requests for multiple files? > > To be clear, I'm not implying that you should do that work all by > > yourself. > > I was actually considering that. The problem I have is that, as > Stephen pointed out, the current protocol must be supported for a > while. As such, that would have to be /filelist/1/. I was imagining that this would simply be a bunch of GET /file/0/ requests pipelined together. We'd need to write a httplib that can issue pipelined requests, as no Python implementations seem to do this yet. I'm also not sure how many requests Apache will serve before it says enough is enough. (Can we request 64 different files at once?) > > catalog.py: > > > > I'm a little slow this morning. Why do we need to embed two different > > methods in catalog.send()? I'm sure it's justified. I'm just having a > > hard time figuring out why. > > The test cases for the api need the ability to output the catalog to a > file directly. > > I can either move the file generation code to the tests and flatten > this function, or leave it as is. > > Either one is fine with me. I don't have a preference. I was just curious and under-caffeinated. > > updatelog.py: > > > > 26 - This module is common to the client and the server. Unless I'm > > being a moron, this import means the client will also depend upon > > cherrypy being installed. > > However, I have worked around this by passing in the request and > response object to the function as arguments. Thanks. > One of the things I struggled with while making these changes was > whether it was more efficient to pass the request and response object > around (and cleaner) or whether it was better to simply use the > singleton object to access them. My guess it that it might be faster to pass the request and response object; however, the difference probably isn't enough to be appreciable. > > repository.py: > > 307 - Does serve_file return a 404 if it can't find the file at the path > > that it has been given? > > If you specify a request path that cherrypy is unable to map to an > object through the "mounted object tree" (see quickstart in depot.py) > it will pass it off to the default page handler if you have one. If > you don't have one it will return a 404. > > So yes, currently the depot code is setup with a custom default page > handler that will return a 404 for unknown pages via face.py:unknown. I guess I may have asked this question obliquely. I was trying to figure out what happens if the client requests as filehash that isn't in the depot. The code in file_0 looks like this: return serve_file(os.path.normpath(os.path.join( self.scfg.file_root, misc.hash_file_name(fhash))), 'application/data') So if fhash isn't in the depot and the path we pass to serve_file doesn't actually name a file, do we get a 404 here or an exception? If I read your response correctly, this is a 404. I just wanted to make sure I understood. Thanks, -j ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/15 <[EMAIL PROTECTED]>:
> Hey Dude,
> Thanks for doing this work. I commented on your notes first, and the
Thank you for reviewing this!
>> * I have not yet done extensive performance testing, nor have I
>> checked for the threading issues that we previously had.
>
> Will you do both before you putback? An extensive test may be above and
> beyond the call of duty; however, some amount of performance and
> threading testing would sure be nice.
You'll find my results very interesting.
Notably, I am happy to report that the threading issues are definitely gone.
As for repository import, well, Danek noted that it was about 20%
faster for his particular case.
See attached for further details.
--
Shawn Walker
"To err is human -- and to blame it on a computer is even more so." -
Robert Orben
Please note that these tests were all run from a single SATA Seagate 250gb hard
drive so my system had the unpleasant job of both serving the files from the
depot and installing them.
The test repository I used was a copy of the 2008.05 repository.
old depot code
==
make test:
as user: (104s, 100s, 103s)
as pfexec: (111s, 111s, 117s)
image-create (3rd run):
---
rm -rf $PKG_IMAGE/* && time pkg image-create -F -a
authname=http://localhost:1 $PKG_IMAGE
real0m0.294s
user0m0.223s
sys 0m0.067s
install SUNWfirefox (3rd run):
--
rm -rf $PKG_IMAGE/*
pkg image-create -F -a authname=http://localhost:1 $PKG_IMAGE && time pkg
-R $PKG_IMAGE install SUNWfirefox
DOWNLOADPKGS FILES XFER (MB)
Completed 51/51 8956/8956 477.44/477.44
PHASEACTIONS
Install Phase13547/13547
real1m59.884s
user0m17.175s
sys 0m7.820s
pkg list -sa (3rd run):
---
time pkg -R $PKG_IMAGE list -sa > /dev/null
real0m10.912s
user0m9.729s
sys 0m0.331s
pkg search -s (3rd run):
time pkg -R $PKG_IMAGE search -r -s $PKG_REPO /usr/bin/firefox
INDEX ACTIONVALUE PACKAGE
path link usr/bin/firefox pkg:/[EMAIL PROTECTED]
real0m0.172s
user0m0.103s
sys 0m0.065s
pkg info -r (3rd run):
--
time pkg -R $PKG_IMAGE info -r SUNWfirefox
Name: SUNWfirefox
Summary: Mozilla Firefox Web browser
State: Not installed
Authority: authname (preferred)
Version: 0.5.11
Build Release: 5.11
Branch: 0.86
Packaging Date: Sat May 3 21:56:14 2008
Size: 65.8 MB
FMRI: pkg:/[EMAIL PROTECTED],5.11-0.86:20080503T215614Z
real0m0.408s
user0m0.275s
sys 0m0.129s
new depot code
==
make test:
as user: (114s, 119s, 117s)
as root: (130s, 130s, 135s)
image-create (3rd run):
---
rm -rf $PKG_IMAGE/*
time pkg image-create -F -a authname=http://localhost:1 $PKG_IMAGE
real0m0.354s
user0m0.256s
sys 0m0.093s
install SUNWfirefox (3rd run):
--
rm -rf $PKG_IMAGE/*
pkg image-create -F -a authname=http://localhost:1 $PKG_IMAGE && time pkg
-R $PKG_IMAGE install SUNWfirefox
DOWNLOADPKGS FILES XFER (MB)
Completed 51/51 8956/8956 477.44/477.44
PHASEACTIONS
Install Phase13547/13547
real1m46.627s
user0m32.920s
sys 0m14.071s
pkg list -sa (3rd run):
---
time pkg -R $PKG_IMAGE list -sa > /dev/null
real0m10.083s
user0m9.751s
sys 0m0.299s
pkg search -s (3rd run):
time pkg -R $PKG_IMAGE search -r -s $PKG_REPO /usr/bin/firefox
INDEX ACTIONVALUE PACKAGE
path link usr/bin/firefox pkg:/[EMAIL PROTECTED]
real0m0.173s
user0m0.103s
sys 0m0.065s
pkg info -r (3rd run):
--
time pkg -R $PKG_IMAGE info -r SUNWfirefox
Name: SUNWfirefox
Summary: Mozilla Firefox Web browser
State: Not installed
Authority: authname (preferred)
Version: 0.5.11
Build Release: 5.11
Branch: 0.86
Packaging Date: Sat May 3 21:56:14 2008
Size: 65.8 MB
FMRI: pkg:/[EMAIL PROTECTED],5.11-0.86:20080503T215614Z
real0m0.408s
user0m0.275s
sys 0m0.128s
old depot threading
===
time pkg -R $PKG_IMAGE search -r -s $PKG_REPO /usr/bin/firefox
--
dtrace -n 'syscall:::entry /execname == "depot.py"/{ @num[probefunc] = count();
}'
dtrace: description 'syscall:::entry ' matched 235 probes
^C
accept1
close 1
lwp_continue
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
2008/5/15 <[EMAIL PROTECTED]>: > Hey Dude, > Thanks for doing this work. I commented on your notes first, and the > code-review feedback is follows that. > >> Notes: >> * The tar streaming now has to write the file to a temporary file >> first before streaming. This is an unfortunate side-effect of cherrypy >> not providing a file object to write data directly back to the client. >> Initially, I considered using a StringIO object, but that would have >> bloated the server process quite a bit (memory usage). > > This effectively negates the intended benefit of streaming the files. > The idea was to write the tar stream out to the socket as we read each > file so that memory and disk usage would remain low. > > The approach that you're taking now doesn't need to employ streaming. > You could just write the tarfile to a temporary file, serve it, and > delete it. I'm not sure that's the right approach, though. Are you > sure there's no sneaky way to get CherryPy to write a streaming response > with an unknown length? The issue is not the unknown length; cherrypy doesn't care about that. The issue is that cherrypy does not provide a file object to write to for the response. tarfile requires that you have a file object to write to from what I can tell. One possible thought I had was to try creating a temporary file and use that for the tarfile object, and after each tar_strem.add(), stream the temporary file and then truncate it. I would repeat that until done. However, I'm not sure if such chicanery would work :-) > In a previous e-mail you noted that it's possible to run CherryPy behind > Apache. The documentation for downloading files seems to hint that > using Apache is preferable, at least for this task. > (http://www.cherrypy.org/wiki/FileDownload). That was one of my concerns and why I tried to ensure that all "static" content was served via /static/. Eventually, if we created /file/1/ and /filelist/1/ that did just GET foo; we could let Apache serve those requests and let the depot continue to handle /filelist/0/ and /file/0/. > There has been a fair amount of hand-wringing about how filelist doesn't > fit with our architectural principles. Perhaps now would be an > opportune time to investigate whether we could switch to sitting behind > Apache and simply pipeline our requests for multiple files? > To be clear, I'm not implying that you should do that work all by > yourself. I was actually considering that. The problem I have is that, as Stephen pointed out, the current protocol must be supported for a while. As such, that would have to be /filelist/1/. >> * I have not yet done extensive performance testing, nor have I >> checked for the threading issues that we previously had. > > Will you do both before you putback? An extensive test may be above and > beyond the call of duty; however, some amount of performance and > threading testing would sure be nice. Yes, and I have been doing some as time goes on. > I have one more question that wasn't really in your notes. One of the > justifications for switching to CherryPy was that it would simplify our > codebase. However, most of the code in the depot seems to have been > moved (more, or less) from depot.py to repository.py. Is there some > portion of the existing code that you think we could simplify once we > switch to using CherryPy? I'm also curious how you see this change > improving our ability to add new features to the depot. Perhaps you > could give an example of how you see this helping future development? Actually, I removed quite a large chunk of code even though I moved it to repository.py. In particular, all of the static file serving code is now gone and the extra code that was required to setup a response or handle exceptions is now gone as well in many cases. For further simplification of output generation (mainly for the status pages or other "web interfaces") a template system is needed and that will come in a later phase. The repository/depot code could use some further restructuring as well which I believe could compact it further. >> webrev: >> http://cr.opensolaris.org/~swalker/pkg-depot-3/ > > depot.py: > > 44 - Would it make sense to check if the import failed and print a message > that CherryPy is required to run the depot? I have made this change. I have also added cherrypy as a dependency to the SUNWipkg pkgdef. > catalog.py: > > I'm a little slow this morning. Why do we need to embed two different > methods in catalog.send()? I'm sure it's justified. I'm just having a > hard time figuring out why. The test cases for the api need the ability to output the catalog to a file directly. I can either move the file generation code to the tests and flatten this function, or leave it as is. Either one is fine with me. > updatelog.py: > > 26 - This module is common to the client and the server. Unless I'm > being a moron, this import means the client will also depend upon > cherrypy being installed
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Hey Dude, Thanks for doing this work. I commented on your notes first, and the code-review feedback is follows that. > Notes: > * The tar streaming now has to write the file to a temporary file > first before streaming. This is an unfortunate side-effect of cherrypy > not providing a file object to write data directly back to the client. > Initially, I considered using a StringIO object, but that would have > bloated the server process quite a bit (memory usage). This effectively negates the intended benefit of streaming the files. The idea was to write the tar stream out to the socket as we read each file so that memory and disk usage would remain low. The approach that you're taking now doesn't need to employ streaming. You could just write the tarfile to a temporary file, serve it, and delete it. I'm not sure that's the right approach, though. Are you sure there's no sneaky way to get CherryPy to write a streaming response with an unknown length? In a previous e-mail you noted that it's possible to run CherryPy behind Apache. The documentation for downloading files seems to hint that using Apache is preferable, at least for this task. (http://www.cherrypy.org/wiki/FileDownload). There has been a fair amount of hand-wringing about how filelist doesn't fit with our architectural principles. Perhaps now would be an opportune time to investigate whether we could switch to sitting behind Apache and simply pipeline our requests for multiple files? To be clear, I'm not implying that you should do that work all by yourself. > * I have not yet done extensive performance testing, nor have I > checked for the threading issues that we previously had. Will you do both before you putback? An extensive test may be above and beyond the call of duty; however, some amount of performance and threading testing would sure be nice. I have one more question that wasn't really in your notes. One of the justifications for switching to CherryPy was that it would simplify our codebase. However, most of the code in the depot seems to have been moved (more, or less) from depot.py to repository.py. Is there some portion of the existing code that you think we could simplify once we switch to using CherryPy? I'm also curious how you see this change improving our ability to add new features to the depot. Perhaps you could give an example of how you see this helping future development? > webrev: > http://cr.opensolaris.org/~swalker/pkg-depot-3/ depot.py: 44 - Would it make sense to check if the import failed and print a message that CherryPy is required to run the depot? catalog.py: I'm a little slow this morning. Why do we need to embed two different methods in catalog.send()? I'm sure it's justified. I'm just having a hard time figuring out why. updatelog.py: 26 - This module is common to the client and the server. Unless I'm being a moron, this import means the client will also depend upon cherrypy being installed. repository.py: Just so I'm sure I understand, what does the @cherrypy.expose decorator do? 79 - If cherrypy handles this automatically, can we assert this condition, or throw some kind of exception if we're processing a function that isn't exposed? 307 - Does serve_file return a 404 if it can't find the file at the path that it has been given? There are a number of places in updatelog and transaction where you refer to the current request or response as cherrypy.request and cherrypy.response, respectively. Are these guaranteed to always be for the HTTP session we're currently processing? Do you know how cherrypy enforces this? Thanks, -j ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Shawn Walker wrote: > On Thu, May 15, 2008 at 11:19 AM, Tom Mueller (pkg-discuss) > <[EMAIL PROTECTED]> wrote: >> Just curious. This continues to use an SVR4 package as an intermediate form >> in publishing an IPS package even though an SVR4 package isn't needed here. >> I understand that this is because the build structure for doing this with >> SVR4 packages is already there. Is there an intent to wean ourselves off of >> SVR4 packages? Should that be now? > > I was assuming that this should still be deliverable on SVR4-based systems. > Which is needed until we can require all machines hosting repos to be running 2008.05 or later. - Bart -- Bart Smaalders Solaris Kernel Performance [EMAIL PROTECTED] http://blogs.sun.com/barts "You will contribute more with mercurial than with thunderbird." ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Thu, May 15, 2008 at 11:19 AM, Tom Mueller (pkg-discuss) <[EMAIL PROTECTED]> wrote: > Just curious. This continues to use an SVR4 package as an intermediate form > in publishing an IPS package even though an SVR4 package isn't needed here. > I understand that this is because the build structure for doing this with > SVR4 packages is already there. Is there an intent to wean ourselves off of > SVR4 packages? Should that be now? I was assuming that this should still be deliverable on SVR4-based systems. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Just curious. This continues to use an SVR4 package as an intermediate form in publishing an IPS package even though an SVR4 package isn't needed here. I understand that this is because the build structure for doing this with SVR4 packages is already there. Is there an intent to wean ourselves off of SVR4 packages? Should that be now? Thanks. Tom Shawn Walker wrote: > I have posted an updated webrev that adds CherryPy to the install and > package build process: > > http://cr.opensolaris.org/~swalker/pkg-depot-3/ > > I have also corrected modules/server/__init__.py to include the > repository module. > > No other changes were made. > > Cheers, > ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Wed, May 14, 2008 at 5:58 PM, Shawn Walker <[EMAIL PROTECTED]> wrote: > [Apologies for breaking threading, I had to fake this reply since > Stephen's email got dropped and I had to dig the response out of the > mail list archives at mail.opensolaris.org] > > On Tue, May 13, 10:00:59 PDT 2008, Stephen Hahn <[EMAIL PROTECTED]> wrote: >>On Mon, May 12, 2008 at 5:30 PM, Shawn Walker <[EMAIL PROTECTED]> wrote: >>> Personally, I like the fact that all of the software is in one package >>> so we are guaranteed to get all of the components. >>> >>> However, I'd be curious as to what others think about this. >>> >>> So, in regards to the "client" having SUNWipkg installed, yes, this >>> does create a dependency on cherrypy and I should probably update the >>> SUNWipkg package definition to note that. >>> >>> I'm also not certain how to handle the cherrypy package. Should I >> > create a definition in the pkg repository as part of this commit? >> >> For now, and because CherryPy seems to be active, it would be best if >> the build process built and installed CherryPy in the proto area. I >> think two packages (ipkg, cherrypy) is most realistic, but would >> settle for one (ipkg). >> >> Since we're going to be receiving PackageManager in our gate, getting >> CherryPy in will serve as a useful example. > > I will update my webrev and try the two package route then tonight; > and see if I can get it installing into the proto area. I have posted an updated webrev that adds CherryPy to the install and package build process: http://cr.opensolaris.org/~swalker/pkg-depot-3/ I have also corrected modules/server/__init__.py to include the repository module. No other changes were made. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Wed, May 14, 2008 at 06:08:42PM -0500, Shawn Walker wrote: > That's the best news I've heard all day. > > I really needed that :-) Glad to help. :) > So, if I understand you correctly, the new depot code is about 20% > faster for this particular case? Looks like it. Don't have numbers for the install times, but I doubt that depot time is that significant there. Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Wed, May 14, 2008 at 6:05 PM, Danek Duvall <[EMAIL PROTECTED]> wrote: > I haven't done a code review yet, but I pulled the bits and did a full > redist_import, which completed successfully, and did so in just under 80% > of the time of the original. Nice! > > Installing the redistributable cluster is progressing just fine, too. At > least, the download completed successfully, and I assume the rest will, > too. That's the best news I've heard all day. I really needed that :-) So, if I understand you correctly, the new depot code is about 20% faster for this particular case? Thanks, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
I haven't done a code review yet, but I pulled the bits and did a full redist_import, which completed successfully, and did so in just under 80% of the time of the original. Nice! Installing the redistributable cluster is progressing just fine, too. At least, the download completed successfully, and I assume the rest will, too. Danek ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
[Apologies for breaking threading, I had to fake this reply since Stephen's email got dropped and I had to dig the response out of the mail list archives at mail.opensolaris.org] On Tue, May 13, 10:00:59 PDT 2008, Stephen Hahn <[EMAIL PROTECTED]> wrote: >On Mon, May 12, 2008 at 5:30 PM, Shawn Walker <[EMAIL PROTECTED]> wrote: >> Personally, I like the fact that all of the software is in one package >> so we are guaranteed to get all of the components. >> >> However, I'd be curious as to what others think about this. >> >> So, in regards to the "client" having SUNWipkg installed, yes, this >> does create a dependency on cherrypy and I should probably update the >> SUNWipkg package definition to note that. >> >> I'm also not certain how to handle the cherrypy package. Should I > > create a definition in the pkg repository as part of this commit? > > For now, and because CherryPy seems to be active, it would be best if > the build process built and installed CherryPy in the proto area. I > think two packages (ipkg, cherrypy) is most realistic, but would > settle for one (ipkg). > > Since we're going to be receiving PackageManager in our gate, getting > CherryPy in will serve as a useful example. I will update my webrev and try the two package route then tonight; and see if I can get it installing into the proto area. Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
* Shawn Walker <[EMAIL PROTECTED]> [2008-05-12 22:31]: > On Mon, May 12, 2008 at 5:24 PM, Tom Mueller (pkg-discuss) > <[EMAIL PROTECTED]> wrote: > > Shawn Walker wrote: > > > > > > > > > > > > Any thought of splitting pkg into client and server packages, since the > > > > server part is taking on this dependency? > > > > > > > > > > > > > > Isn't it already? I don't see anything here that would cause the > > > client to take on this dependency. Only depot.py. > > > > > > > > > > > All of the IPS code is currently in SUNWipkg. There isn't any formal split > > within IPS between what is used by the client and the server. Generally, > > the pkg.client is used by the client (pkg), pkg.publish is used by pkgsend, > > pkg.server is used by depot.py, and pkg.actions, pkg.* pkg.portable are used > > by both. However, I'm not sure if splitting things up this way would > > actually be that easy. > > Sorry, when I read packages my brain said python modules. > > Personally, I like the fact that all of the software is in one package > so we are guaranteed to get all of the components. > > However, I'd be curious as to what others think about this. > > So, in regards to the "client" having SUNWipkg installed, yes, this > does create a dependency on cherrypy and I should probably update the > SUNWipkg package definition to note that. > > I'm also not certain how to handle the cherrypy package. Should I > create a definition in the pkg repository as part of this commit? For now, and because CherryPy seems to be active, it would be best if the build process built and installed CherryPy in the proto area. I think two packages (ipkg, cherrypy) is most realistic, but would settle for one (ipkg). Since we're going to be receiving PackageManager in our gate, getting CherryPy in will serve as a useful example. - Stephen -- [EMAIL PROTECTED] http://blogs.sun.com/sch/ ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Mon, May 12, 2008 at 5:58 PM, Jordan Brown (Sun) <[EMAIL PROTECTED]> wrote: > Shawn Walker wrote: > > > Personally, I like the fact that all of the software is in one package > > so we are guaranteed to get all of the components. > > > > However, I'd be curious as to what others think about this. > > > > Assuming that the software in question is relatively small and contains no > setuid components... > > Sure, that sounds great, but what about dependencies? A client shouldn't > depend on having a web server, and dragging around yet another standalone > web server doesn't sound like a win. > > Some people might complain about clutter in their SMF service lists. I guess I don't see it as dragging around a web server. It also depends on how we end up handling client installs from on-disk repositories. -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Shawn Walker wrote: > Personally, I like the fact that all of the software is in one package > so we are guaranteed to get all of the components. > > However, I'd be curious as to what others think about this. Assuming that the software in question is relatively small and contains no setuid components... Sure, that sounds great, but what about dependencies? A client shouldn't depend on having a web server, and dragging around yet another standalone web server doesn't sound like a win. Some people might complain about clutter in their SMF service lists. ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Mon, May 12, 2008 at 5:24 PM, Tom Mueller (pkg-discuss) <[EMAIL PROTECTED]> wrote: > Shawn Walker wrote: > > > > > > > > Any thought of splitting pkg into client and server packages, since the > > > server part is taking on this dependency? > > > > > > > > > > Isn't it already? I don't see anything here that would cause the > > client to take on this dependency. Only depot.py. > > > > > > > All of the IPS code is currently in SUNWipkg. There isn't any formal split > within IPS between what is used by the client and the server. Generally, > the pkg.client is used by the client (pkg), pkg.publish is used by pkgsend, > pkg.server is used by depot.py, and pkg.actions, pkg.* pkg.portable are used > by both. However, I'm not sure if splitting things up this way would > actually be that easy. Sorry, when I read packages my brain said python modules. Personally, I like the fact that all of the software is in one package so we are guaranteed to get all of the components. However, I'd be curious as to what others think about this. So, in regards to the "client" having SUNWipkg installed, yes, this does create a dependency on cherrypy and I should probably update the SUNWipkg package definition to note that. I'm also not certain how to handle the cherrypy package. Should I create a definition in the pkg repository as part of this commit? Cheers, -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Mon, May 12, 2008 at 4:32 PM, Shawn Walker <[EMAIL PROTECTED]> wrote: > On Mon, May 12, 2008 at 4:23 PM, Tom Mueller <[EMAIL PROTECTED]> wrote: > > In repository.py, I don't see where usage() is used. > > It isn't. It is used in depot.py. repository.py is strictly intended > to be for the depot server at the moment. Sorry Tom, I'm a bonafide idiot :-) I just realised what you meant by it not being used. That's a leftover from when I moved the functionality in depot.py to repository.py. I also noticed that the "sys" and "StringIO" imports were unused and removed them. Thanks for your sharp eye on this. I have posted an updated webrev (only repository.py changed): http://cr.opensolaris.org/~swalker/pkg-depot-2/ -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Shawn Walker wrote: > >> Any thought of splitting pkg into client and server packages, since the >> server part is taking on this dependency? >> > > Isn't it already? I don't see anything here that would cause the > client to take on this dependency. Only depot.py. > > All of the IPS code is currently in SUNWipkg. There isn't any formal split within IPS between what is used by the client and the server. Generally, the pkg.client is used by the client (pkg), pkg.publish is used by pkgsend, pkg.server is used by depot.py, and pkg.actions, pkg.* pkg.portable are used by both. However, I'm not sure if splitting things up this way would actually be that easy. Tom ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
On Mon, May 12, 2008 at 4:23 PM, Tom Mueller <[EMAIL PROTECTED]> wrote: > Shawn, > > What were your thoughts about getting Cherrypi onto the system? I don't see > a package for it currently in OpenSolaris. Would it just be bundled with > pkg? A separate package? Merged with the python package? To support pkg on > other platforms, we'll have to get Cherrypi there too, which means packaging > it somehow. Well, I hadn't though of it, but I can certainly create a package. > Are you just setting PYTHONPATH to do the build? Yes. I did a "python setup.py install --home=~" and then set my PYTHONPATH. > Any thought of splitting pkg into client and server packages, since the > server part is taking on this dependency? Isn't it already? I don't see anything here that would cause the client to take on this dependency. Only depot.py. > In repository.py, I don't see where usage() is used. It isn't. It is used in depot.py. repository.py is strictly intended to be for the depot server at the moment. -- Shawn Walker "To err is human -- and to blame it on a computer is even more so." - Robert Orben ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Re: [pkg-discuss] Code review request for [bugs 1154, 1237, 1845, 1887, 1888]
Shawn, What were your thoughts about getting Cherrypi onto the system? I don't see a package for it currently in OpenSolaris. Would it just be bundled with pkg? A separate package? Merged with the python package? To support pkg on other platforms, we'll have to get Cherrypi there too, which means packaging it somehow. Are you just setting PYTHONPATH to do the build? Any thought of splitting pkg into client and server packages, since the server part is taking on this dependency? In repository.py, I don't see where usage() is used. Thanks. Tom Shawn Walker wrote: The following webrev includes proposed fixes for the following bugs: 1854 rework depot to use higher-level framework 1154 pkg.depotd tracebacks when given bad options 1237 pkg.depotd -p 80 -p 90 should return usage, but second option is taken 1887 depot status page output is not valid HTML 1888 memleaks test overwrites PYTHONPATH webrev: http://cr.opensolaris.org/~swalker/pkg-depot/ Notes: * The tar streaming now has to write the file to a temporary file first before streaming. This is an unfortunate side-effect of cherrypy not providing a file object to write data directly back to the client. Initially, I considered using a StringIO object, but that would have bloated the server process quite a bit (memory usage). * This is the most python I have ever written. Please excuse any beginner's mistakes and be sure to watch out for them. * This commit will cause cherrypy to be a *required* component for the ips depot server. * The tests take about 15-20 seconds longer to complete now. * pylint was run and all reasonable warnings and errors were corrected for files I changed. * I have not yet done extensive performance testing, nor have I checked for the threading issues that we previously had. * You can cherrypy applications "behind apache" [1] -- and using a reverse proxy is the recommended configuration to do so. * Yes, cherrypy supports SSL, and if you run it behind Apache, let Apache handle all the work [2]. * I'm expecting lots of feedback :-) * Thanks to Danek for providing me the repository data that I needed to do some of my own testing. * Creating my own image, and then using the repo to do "pkg install SUNWfirefox" was entertaining to say the least. It took six minutes for the download portion, and one minute for install phase and operation completion. Cheers, begin:vcard fn:Tom Mueller n:Mueller;Tom org:Sun Microsystems, Inc.;Update Center/OpenInstaller Software adr:;;21915 Hillandale Dr;Elkhorn;NE;68022;USA email;internet:[EMAIL PROTECTED] title:Senior Staff Engineer tel;work:877-250-4011 tel;fax:877-250-4011 tel;home:402-916-9943 x-mozilla-html:TRUE version:2.1 end:vcard ___ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
