Shawn Walker wrote:
Brock Pytlik wrote:
Shawn Walker wrote:
Greetings,
The following webrev contains changes and fixes for the following
issues and RFEs:
5872 List APIs required
12929 image.py:get_publisher_ranks can traceback after older
client configuration changes
12946 pkg list should provide way to list only newest versions of
all packages
webrev:
http://cr.opensolaris.org/~swalker/pkg-list/
[snip]
High level comment, I'm not sold on the idea of "
Packages that do not apply to the current
image (such as those for a different architecture or that are
not applicable to the current zone type) will never be listed.
"
I'd kinda like an option to say, no, I really mean ALL packages.
Mostly, I'm thinking about a case where a user has two images in two
different situations (one on sparc, one on x86 or one that's a global
zone and one that's a non-global zone) and one is just really
hosed and they're trying to use the other to gather information about
what packages are available that might be missing.
So you're ok with architecture filtering, but not zone variant
filtering then by default?
I'm of the belief that most users are not interested in packages that
are not for their image variant by default.
However, I did make it so that its trivial to include packages not for
the current variant with the API.
I could easily add a command-line option that causes the cli to
include variant packages. Would this suffice, and if so, what should
that option be?
I'm fine with everything you're doing by default. All I wanted was a way
for a user to get all the packages, regardless of variants/applicability.
As for what the option should be, I don't really care much. We've used
the obvious ones like -a, -v. One possibility would be -V. Whatever you
decide is fine, I'm not going to bikeshed the letter choice.
client.py:
I'm not sure the division between client and api here is quite right.
I think more of the logic here should be moved into the api.
For example, the items on lines 294-308 I would think should be
within the logic for get_pkg_list? Wouldn't the gui want to refresh
when the cli would?
I tend to disagree here. The API provides a refresh function, and
adding a refresh function to get_pkg_list would then require a
transport lock. Since clients can simply call refresh() themselves if
they so desire, I saw no need to combine two different API functions.
And actually, no, the GUI actually wouldn't. I suspect different
clients have vastly different refresh behaviour. For example, the GUI
refreshes on startup and various other places after operations
complete and so on. It also triggers refreshes through the
updatemanager.
Ok, then can we call the api refresh instead of img.refresh? (though it
sounds from what you've said below that you've already done this)
I know it's a nit, but could you rename pkg_list to pkg_selection or
something like that?
324: I'd prefer we make this function (get_preferred_publisher) part
of the api instead of leaving this stray reference directly to the
image object which I thought we wanted to remove from clients of the
api. Eventually, I'd like us to pass a api_inst instead of an image
object to functions like list_inventory or search or list_contents so
that we don't have this dual mode of image and api objects.
Actually, it is already part of the API and I had simply forgotten to
convert the rest of the calls in list_inventory to API calls. I've
done that now. All of the history and pref publisher stuff is already
in the API.
[snip]
version.py:
468-484: why not return False to break out early if any of these
tests fail?
For example, instead of lines 465-469, why not:
if not ((other.release and self.release and
other.release.is_subsequence(self.release)) or not other.release or
other.release == '*'):
return False
I guess with all the other performance changes, I'm surprised this
one wasn't made.
Sort of got tunnel vision here; it was this way before so I left it
that way. I've changed it though, although I've seen no appreciable
difference in performance (oddly enough).
Huh, well if it doesn't make a measurable difference, feel free to do it
either way, whichever is clearer code.
t_pkg_list:
Given that we know have t_api_list, are there tests her that could be
removed, or moved into t_api_list which is likely faster? (I'm fine
with this just being filed as a bug for future consideration.)
Some of these possibly, although this test suite completes fairly
fast, and we have to test that the cli is using the API correctly so
gives the expected results. I'll file a bug for investigation soon.
This is something I've frequently wrestled with in writing unit tests
(avoiding unnecessary test duplication).
I understand. What I did with search was have a few cli tests that
covered the output and the command line formats, and (eventually) move
most of the rest of the tests to be api tests instead.
Brock
Cheers,
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
