[email protected] wrote:
Change Summary:
---------------
* Added new --force option to pkg refresh subcommand
* Added 'force' parameter to pkg.client.api.ImageInterface.refresh() to
allow forcing an incremental refresh of publisher metadata.
It doesn't make sense to have the --force option. I agree that we
should only implicitly refresh the catalog after refresh interval has
elapsed; however, if the user requests a refresh, they shouldn't have to
type refresh -f (or --force) to get the desired behavior.
I agree with that possibly in the case where a specific publisher is
being refreshed, but I'm not certain about the general refresh request
case. I'm a little torn here, although I could see the argument for
only using the automatic update interval logic for the refreshes we
perform internally, while an explicit refresh request *always* uses
force=True in the api.
webrev:
http://cr.opensolaris.org/~swalker/pkg-7587/
publisher.py:
- lines 806-848: This construction seems awkward. How about
re-writing this as:
...
While we're on the subject of this function, I also don't think it
makes a whole lot of sense to always call makedirs. I would think
that you would want to move this case info the open(lcfile)'s
exception handler. If you extract the code starting a f = open up
to f.close() into its own function, it won't be so hard to call this
a second time from the exception handler.
I've reworked this function based on your feedback.
image.py:
...
- line 1438: Why is this separate from retrieve_catalogs, and what
does it mean to refresh a publisher? The catalog is what we're
trying to refresh.
Because, currently, refresh_publishers() does some things that don't
work at the point of image-creation or the initial add of a publisher.
retrieve_catalogs() is intended to be an internal function used by the
image in cases where it knows no data exists to be refreshed. I'd need
to rework several other functions to not make certain assumptions about
the state of the image to merge refresh_publishers() and
retrieve_catalogs(). If you think that's preferable, I can rework this
further.
In answer to your second question, refreshing a publisher means
refreshing *all* of its metadata. At the moment, we only have the
catalog, but eventually that will mean performing a publisher/0 query on
the repository and updating the information about the publisher and the
repository as well.
I'm also concerned about accumulating random disjoint policy in
different places in the image object. Are there any parts of this
function that can be extracted and moved to the publisher object?
I'm not certain what you mean by disjoint policy at the moment as far as
I can tell, but my hope was to *eventually* move more of the metadata
management that the image performs now to the publisher object.
- line 1590: Let's not add XXX comments, if possible. I know how to
handle this case. The catalog's origin must be in the set of origin
urls, instead of matching just one. This is another reason why I
was asking you about a straight-forward mechanism to get all of a
publishers origin URLs either as an iterator or list.
Then:
if cat.origin() not in repo.origins:
...works as well since I have __eq__ methods for RepositoryURI objects.
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
