On 11/08/12 05:40, Brock Pytlik wrote:
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.
Modifying the publisher object or removing the origins would break
*Bug 18105* <http://defect.opensolaris.org/bz/show_bug.cgi?id=18105>
-api should support multiple repositories (origins) with different
package data
WRT the user specifying two origins ,it is handled by the for loop which
populates slist.
Therefore I would go with the approach of providing the altrepo.
The new webrev is create @
https://cr.opensolaris.org/action/browse/pkg/tk241774/15744194-rev02/webrev/
Note: I have included the fix to bug "15847480 Space required after
line 2779 in api_errors.py"
- 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.
Agreed !!. I have included it in the webrev.
- 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.
Agreed. Made changes accordingly.
Brock
Hope I answered your queries...
Thanks,
Thejaswini K.
Brock
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss