Hi Shawn, On Thu, Feb 04, 2010 at 03:16:02PM -0600, Shawn Walker wrote: > >>webrev: > >>http://cr.opensolaris.org/~swalker/pkg-11522/ <...> > >api_errors.py: > > > > - lines 1249-1256: I believe this exception is redundant. > > UnsupportedRepositoryOperation is what the Transport currently uses > > when it cannot find any repositories from the requested publisher > > that support a particular operation. > > Yes and no. I originally had it as a subclass of > UnsupportedRepositoryOperation but that's not quite right for a few > reasons: > > * it also gets raised when a repository supports the operation but > doesn't provide any configuration information) > > * it doesn't use the additional arguments normally passed to > UnsupportedRepositoryOperation > > I suppose the other option here would be to raise both exceptions > separately, but I'm not really happy with either direction; > suggestions are welcome.
After reading the first three paragraphs of your response, my thought was that perhaps it would be fine to raise each exception separately. In one case, none of the configured repositories support the requested operation. In the other case, the operation is supported but the needed information wasn't returned. If you really don't think that the two cases are different, I don't have a problem with you changing UnsupportedRepositoryOperation so that you could subclass it with RepoPubConfigUnavailable. It's currently only used by the catalog1 path, and it would be simple to change the arguments there, I think. Do you return an InvalidP5IFile exception for cases where the P5i file parsed, but it didn't return any configuration information? In my previous comments I had suggested you always declare this case a content error, but if your got valid content without valid configuration, the exception handling code should probably make a distinction. > >transport/transport.py: > > > > - Does your code use both get_publisherdata and get_publisherinfo? > > At the moment, I'm only using the get_publisherdata(); I reserved > get_publisherinfo() for later. One's to get the raw information and > the other is to get the processed information. Ok. I was going to ask you to please remove the unused function, but if you have something in the pipe that's going to make use of it, then it's fine to keep it for now. I was just trying to avoid bitrot. Thanks, -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
