[email protected] wrote:
Brock,
Webrev:
http://cr.opensolaris.org/~bpytlik/ips-6712-v1/
Bug:
6712 search shouldn't index actions for other variants of an image
http://defect.opensolaris.org/bz/show_bug.cgi?id=6712
Just a few questions:
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?
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.
- 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.
Looks good otherwise.
Thanks for the quick feedback.
-j
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
