>> indexer.py: >> >> - line 396: Is this redundant? Or is fmri_list not a list? If not, >> should we name it something less confusing? >> > What ends up reaching there is a generator of fmris. I'm fine with changing > it. Would 'fmris' work?
Ah, I somehow missed that. How about fmri_gen, or something that indicates it's a generator function? >> server/catalog.py: >> >> - line 291: Do we need EnvironmentError exception handling here? >> > Hmm, probably. What's the right behavior for a depot which is missing one > or more manifest files, and what's the right search behavior? Without > looking too deeply into the code, my guess is that the right thing to do is > to just drive on and serve the rest of the information as search results, > and log some kind of error. There's no (simple/easy/reasonable) way for the > indexing process, which is (usually) a different process than the one > serving requests, to tell the other process to stop serving all search > requests, and I'm not even sure if that's the behavior we'd want anyway. At least for now, logging an error is probably sufficient. My guess is that if a depot is missing a manifest, the administrator is going to get a lot of complaints from users. My concern was that we'd have a stack trace and the indexing would never finish. >> - Nit: It seems a little confusing to have both image.get_manifest and >> catalog.get_manifest. After looking at server/catalog.py, I now >> understand why you made this an argument to the indexer. Should >> these functions have different names to make it a bit clearer that >> one belongs on the serverside and the other belongs to the image? I >> understand that we can infer this by looking at the object that's >> defining the function, but I thought I'd ask anyway. >> > I'm fine with a different name. catalog.get_catalog_manifest? > catalog.get_server_manifest? Let me know which is preferred. Or image.get_manifest and server.catalog.get_server_manifest. This is just me being anal. You don't have to change this if you don't want to. -j _______________________________________________ pkg-discuss mailing list pkg-discuss@opensolaris.org http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
