On 05/22/12 02:29, Tim Foster wrote:
Hi Shawn,
Thanks for the review, much appreciated.
On 05/14/12 08:18 AM, Shawn Walker wrote:
On 05/03/12 19:29, Tim Foster wrote:
https://cr.opensolaris.org/action/browse/pkg/timf/granular-proxy-v2
...
src/modules/client/api.py:
This change needs an API version bump, even though it's compatible.
Please also update doc/client_api_versions.txt.
I've made this change. I had thought we discussed this before and
decided that it didn't warrant an API version change (perhaps I'm
confusing it with the conversation about ImageConfig versioning)
I think there's confusion here. Any time the API changes (that is, the
set of parameters passed into functions or new functions are added or
removed), we bump the version. The only difference is how the list of
compatible versions changes.
...
src/modules/client/publisher.py:
...
lines 613, 630, 658, 674, 723, 739, 767, 784:
These functions have had their operating semantics changed
incompatibly; previously, they would match solely on the URI.
hmm...
Now objects have to match both on proxy and URI. That's not
right. If I call remove_origin() with a single URI, then it
should remove any origins with that URI regardless of proxy.
Look at __eq__ and __ne__ in RepositoryURI.
I disagree. It's not that the semantics have changed, it's that the
identifying features of an origin/mirror have changed.
This is not a compatible change; the removal functions no longer remove
an origin if you don't also specify a matching proxy; that seems wrong.
I made the changes to __eq__, __ne__ and __cmp__ to ensure that when
removing an origin or mirror, you must specify the proxy and URI in
order to uniquely identify the RepositoryURI object we're removing (and
updated api_errors.UnknownRepositoryOrigin to account for proxy values)
Not specifying a --proxy flag implies a proxy value of 'None.
Before now, all we needed to identify an origin was its URI, because
that's the only property it had. (since ssl keys/certs are publisher
properties at present, rather than origin properties)
granular$ pkg set-publisher --no-refresh --proxy http://baz -g
http://foo solaris
granular$ pkg set-publisher --no-refresh --proxy http://zoo -g
http://foo solaris
granular$ pkg set-publisher -G http://foo solaris
pkg set-publisher: Unknown repository origin 'http://foo/'
granular$ pkg set-publisher -G http://foo --proxy http://noodles solaris
pkg set-publisher: Unknown repository origin 'http://foo/' with proxy
'http://noodles'
The intent of this changeset is to tie the proxy and URI closely
together because they both configure the transport-path to our repository.
Otherwise, when trying to preserve the old behaviour, it would be
impossible to remove just an unproxied URI and leave a proxied URI
configured, since with the configuration:
# pkg set-publisher -g http://foo solaris
# pkg set-publisher -g http://foo --proxy http://bar solaris
"pkg set-publisher -G http://foo solaris"
would remove both the proxied and non-proxied origins. The exception is
'-G *' which removes all origins.
I believe that this is not the right direction to go. As a user, I do
not expect to have to specify the matching proxy for an origin to remove
the origin.
I also remain unconcerned about a user being able to selectively remove
old, proxied URIs. The only proxied URIs we would have had in the past
would have been from the system repository.
With that in mind, I can't imagine why I'd want to have both a proxied
and un-proxied version of an origin as a user. Whether we need to do
that for the system repository seems irrelevant to the user, and I think
this significantly complicates our interface.
The API and user interface both need to continue to work as they used to
here; if you specify an origin to remove, then remove the matching
origin. It also shouldn't be possible to have both a proxied and
unproxied version of the same origin. The origin URI should be the
unique identifier for each repository.
Please rework this.
...
What happens when the packagemanager tries to manipulate origins/mirrors
that have granular proxies set?
Good question. Given a publisher with two origins with identical URIs,
one proxied, one not, it'll allow you to remove the unproxied origin,
but not the proxied one, since packagemanager doesn't ask for the
required proxy information to properly select the origin from the list.
We need to fix that; see above.
Then, trying to add back the unproxied origin, it'll tell you the URI is
already in use.
Otherwise, PackageManager appears to work fine - it can install,
uninstall and list packages from proxied publishers.
...
I didn't see anything in here to update .p5i to allow proxy to be
specified. How do you want to handle that?
I think we had this conversation before, I was initially thinking about
adding the proxy to the p5i format, but it was suggested that this could
be a bad idea:
http://mail.opensolaris.org/pipermail/pkg-discuss/2012-March/028807.html
suggesting that a server shouldn't be able to specify the proxy used for
its clients (unless I'm misunderstanding your question?)
Well, certainly it's a bad idea for the *server* to use p5i information
to do that, but as a general mechanism for moving transport
configuration from system to the next, I think it's desirable.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
