On 11/07/12 01:09, Thejaswini wrote:
Hi Brok,
Thanks for looking into the webrev.
Please find my answers inlined.
Oracle
Thejaswini K
Revenue Product Engineering (RPE), Systems
Phone: +91 8066927709 | Mobile: +91 9663324594
ORACLE India | Off Langford Road | Bangalore | 560025
Green Oracle <http://www.oracle.com/commitment> Oracle is committed to
developing practices and products that help protect the environment
On 11/07/12 01:08, Brock Pytlik wrote:
On 11/05/12 21:43, Thejaswini wrote:
Folks,
Please review the fixes for the below CR's:
1. 15822242 update-refresh.sh produces noise via cron email
2. 15744194 search -s with a url shouldn't look at other urls
3. 15807844 pkg search for temporary sources fails without
configured publisher
The webrev is @
https://cr.opensolaris.org/action/browse/pkg/tk241774/15744194/webrev/
Thanks,
Thejaswini K.
I've only looked at the search changes.
General comments:
Please wrap lines at 80 characters. It looks to me like several lines
run beyond that limit.
Will take care of it.
I'm confused by the general approach taken in api.py. There's already
a servers argument coming in that defines what urls/servers should be
examined. That list gets transformed into slist, which is iterated
over when calling transport.do_search. In short, I think the approach
taken here isn't the right one. I think the first step is to
determine why transport.do_search, when -s is given on the command
line, is being called on urls other than that single url.
This change is for bug 15744194:search -s with a url shouldn't look at
other urls
When there are two repositories with same publisher name configured on
the system, there are two cases:
1. When pkg search is called without -s
2. When pkg search is called with -s
In case 1:
- The incoming argument 'servers' is empty and its get populated
in the api @ line 4180 and 'servers' become publisher object.
- Then slist is created and passed to transport.do_search and
everything works well..
In case 2: [ Without my changes ]
- The incoming argument 'servers' is a dict containing the path to
the repo uri.
- We get the publisher object ' pub' @ 4214 and populate slist and
pass it to transport.do_search.
- This pub object includes both the repos because they are with
same publisher.
Ok, I think this is where the fix needs to be aimed. The simplest fix is
probably to set the altrepo in slist to the be repo that corresponds to
the origin the user provided. I think that might fix the bug. The
alternative would be to modify the publisher object so that it only has
the relevant repository(ies) set in it. You'll need to think carefully
if you use the "simple" fix I described so that if the user specifies
two origins which point to origins which are both origins for the same
publisher, we handle it correctly. Given that, I'd probably suggest
removing origins from the repository object contained inside the
publisher object is the right way to go.
- When this object is passed to transport.do_search, it searches
in the first listed repo within pub and if it does not find any match
it exits raising negative search error.
This should also be fixed. My first take is that we need to delay
raising a NegativeSearchResult exception until all repos for a publisher
(as determined by gen_repo) have been searched and all returned no content.
- search is not performed for the second repo, and search displays
no results if this repo was specified with -s.
- My fix would make transport.do_search() searching for query in
repos that are only mentioned by -s.
api.py:
4219: Why is it ok to append a tuple whose repo is None here, but we
go through the trouble of creating a repo at 4224?
This change is for bug 15807844: pkg search for temporary sources
fails without configured publisher
Note: This bug fails during /_captive_portal_test/
Line 4224, This part of code executes when the specified repo is not
configured on the system i.e creation of publisher object fails.
We create the repository object from the Repouri object. The
repository object created is passed as alt_repo to do_search.
Therefore the captive_portal_test succeeds and the search result is
displayed.
At line 4219 we need not pass the alt_repo therefore we append None to
the tuple.
t_pkg_search.py:
996-997: This isn't the way to create an image with two origins for
the same publisher. You've created an image with one origin for the
publisher, then overwritten that image with a second image with the
other origin for the publisher.
I tested the testcase :-) by changing the uri provided to -s option
between durl1 and durl2 and I observed the right behavior.[Note:
Without fix for 15807844 ]
Therefore it looks like both the publishers are configured on the same
image. Not sure how!!
If this is not the right way, please let me know how to create an
image with two origins?
Try running pkg publisher on the image you created and looking at the
output. I strongly suspect you won't see what you expect. To create an
image with two origins, first create an image with one origin, then use
pkg set-publisher to add an additional origin to the image.
Brock
Hope I answered your queries...
Thanks,
Thejaswini K.
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
