On 05/23/12 16:58, Tim Foster wrote:
Hi there,
On 05/23/12 01:24 PM, Brock Pytlik wrote:
On 05/22/12 18:10, Tim Foster wrote:
Alright, so this could be a simple user-interface change on my part.
I could make:
# pkg set-publisher -G http://foo bar
delete all instances of http://foo (ie. preserve the current
behaviour) and also make:
I'm not a fan of this change, though I could live with it. Another
suggestion would be this:
1) If a single origin with the uri http://foo is specified (with or
without proxy) -G http://foo removes it
2) If multiple origins with the uri http://foo are specified but one
doesn't have a proxy, -G http://foo removes the unproxied one
3) If multiple origins with the uri http://foo are specified but all
have a proxy, -G http://foo is an error.
So from discussions in the meeting today, I think we reached a consensus.
It was suggested that we simply ensure the user has supplied enough
information so that we can derive their intent, but to continue to only
remove a single origin when a URI is provided on the command line.
This is basically what Brock had suggested above. To rephrase,
1. if single proxied origin exists, but the user doesn't use --proxy
when trying to remove it, we remove the proxied origin so long as it is
the only origin that uses that URI.
2. if multiple proxied origins exist for a single URI and the user
hasn't used --proxy and no matching unproxied origins exist, then we
report an error.
This brings me to something that wasn't accounted for in this set of
changes: composition. As far as I can tell, with the current
implementation, composition breaks as soon as a proxy is configured for
one of the origins. This is because composition was designed with the
assumption that RepositoryURI objects (origin objects) for a publisher
can be matched solely based on URI. And that only one object would have
a matching URI.
If you look at pkg.client.image.Image.get_pkg_repo(), you'll see that it
checks for all pub.repository.origins that have a matching URI by
performing an 'x in list'-style check. Since __eq__ has been completely
changed in pkg.client.publisher, this no longer works as expected.
So a new unit test is definitely needed for composition and proxied
sources. To trigger composition behaviour, you need two or more http
repositories configured as origins for a publisher that contain
different sets of packages. You should then set one of the origins to
be proxied, preferably with alternate proxy configurations, to verify
the results.
The other issue (which I touched on briefly in my other reply) is that
pkg.client.publisher for catalog composition purposes assumes that all
repository.origins for a publisher are reachable. So multiple proxied
sources doesn't make much sense with our current architecture, since the
only reason I can see to do that is because sometimes different proxies
are needed to reach an origin.
In addition, when multiple proxies are involved, content could differ,
and composition assumes that it can store and manage catalogs for a
publisher's origins based on a hash of the origin URI alone. Changing
that would require an incompatible image format rev.
[ item 2. above]
$ pkg set-publisher --no-refresh -G http://foo test
pkg set-publisher: Repository origin 'http://foo/' matches more than one
origin
To add to Danek's other comment, I would say that to the user that's not
another origin -- it's the same origin with different sets of configuration.
So the error might be less confusing as "Repository origin 'http://foo/'
has multiple proxy configurations. To remove all configurations,
--proxy \* must also be specified."
...in short, try to lead them to the solution for removing all matching
proxy configurations. I'm not thrilled with that wording either, but
I'm looking for something else here.
Keep in mind that most of our users still think that publisher ==
repository == single uri. They're confused enough by the possibility of
composition as it is.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
