Danek Duvall wrote: > On Mon, Aug 18, 2008 at 11:21:28AM -0500, Shawn Walker wrote: > >> http://cr.opensolaris.org/~swalker/pkg-2905/ > > What's the point of setting fpath to None first?
Habit in avoiding pylint, obviously not necessary here. Fixed. > Perhaps the except statement should be more specific? (IndexError, > IllegalDotSequence, IllegalVersion) I initially took that approach, but abandoned it for reasons I explain later. > I'm assuming, also, that we do the right thing when the file actually isn't > on disk (as would happen when we give it a partial version)? serve_file() > appears to throw a cherrypy.NotFound() exception, but does something on the > other side of the stack handle that and return a 404 for us? CherryPy already Does The Right Thing(TM) in that case. > I also wonder if, given that we wouldn't expect a malformed fmri to work, > that perhaps 400 is a better response than 404? It's true we didn't find > it, but not just because it wasn't there, but because it couldn't be there. > We can change this later, if we decide to make the client handle that case > more specifically, but it's something to think about. I initially went for the 400 Bad Request response using specific exceptions. However, from a RESTful perspective, isn't a 404 more appropriate (since they have to provide a full fmri)? It would seem to me that if we were serving /manifest/ via a "standard" web server or cdn, that we should just treat it is a "resource request." So, malformed uris simply don't exist (404). Agree, disagree, torn as I am? Change sumary: * Removed unnecessary fpath assignment * Added fix for: 2935 pkg.depotd traceback when unsupported operation version requested Original webrev: http://cr.opensolaris.org/~swalker/pkg-2905/ Updated webrev: http://cr.opensolaris.org/~swalker/pkg-2905-2935/ Diff from last webrev: http://cr.opensolaris.org/~swalker/pkg-2905-2935/pkg-2905-1-2.patch Cheers, -- Shawn Walker _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
