On 04/25/12 09:14 AM, Brock Pytlik wrote:
On 04/23/12 20:54, Tim Foster wrote:
https://cr.opensolaris.org/action/browse/pkg/timf/granular-proxies-webrev-v1/
I thought there was going some indication that this uri was being
proxied?
.
.
I think it makes sense to include that information in the default output
since, imo, even if the text of the URL is the same, if one URL is
proxied and one isn't, they're really different entities.
Yes, that's true - they are different entities.
Even with the -o support, I'd still want the proxy column to be
there in the default
output. What I could live with is that we only show the proxy column
at all if there are any origins with proxies.
Modifying the number of columns based on whether a property is present
or not seems liable to break scripting (yes I know pkg command output is
not an interface, it just seems impolite to change the number of columns
based on the image configuration) which means we get pretty ugly default
output:
(I'm pasting this as a quotation in thunderbird in an attempt preserve
the formatting, not sure if this will work)
timf@kura[120] pkg publisher
PUBLISHER TYPE STATUS PROXY
URI
solaris origin online -
http://ipkg.us.oracle.com/solaris11/dev/
extra origin online -
http://ipkg.us.oracle.com/opensolaris/extra/
internal-only origin online -
http://ipkg.us.oracle.com/internal/solaris11/internal-only/
solarisstudio origin online -
http://ipkg.us.oracle.com/solarisstudio/support/
on-extra origin online -
http://ipkg.us.oracle.com/internal/solaris11/on/extra/
userland origin online -
file:///home/timf/projects/userland/userland-mod_wsgi.hg/i386/repo/
test (disabled)
That looks a bit unweildy imho.
Zones pkg publisher changes
---------------------------
We get rid of the "proxy://<uri>" resources for the short-form pkg
publisher output, instead using the more explanatory
"<system-repository>" value, but provide the full URI in both the
long-form pkg publisher output, as well as the --format tsv output.
.
.
Do you mean, have tsv format use "URI" (which can contain
"<system-repository>" values), and add "Actual URI" (or somesuch) and
"Proxy"?
Yes, I think that's roughly what I meant. I don't like the idea that the
URI column of pkg publisher contains<system-repository> while the URI
column of pkg publisher -F tsv contains proxy://http://.... (or whatever).
Ok, so here's how it could look otherwise - this is simply exposing the
URIs in the zone, rather than dressing them up as "<system-repository>"
We could probably trim the width of the "PROXY" field a little, but I
still prefer to omit it. The line-width looks excessive.
(again pasting as quotation)
# pkg -R /space/image publisher
PUBLISHER TYPE STATUS PROXY
URI
solaris (syspub) origin online http://localhost:1008
http://ipkg.us.oracle.com/solaris11/dev/
extra (syspub) origin online http://localhost:1008
http://ipkg.us.oracle.com/opensolaris/extra/
internal-only (syspub) origin online http://localhost:1008
http://ipkg.us.oracle.com/internal/solaris11/internal-only/
solarisstudio (syspub) origin online http://localhost:1008
http://ipkg.us.oracle.com/solarisstudio/support/
on-extra (syspub) origin online http://localhost:1008
http://ipkg.us.oracle.com/internal/solaris11/on/extra/
userland (syspub) origin online -
http://localhost:1008/userland/614bdd284706b4be9743fa5021c76a968192c904/
blah
baz origin online -
http://other/
# pkg -R /space/image publisher -F tsv
PUBLISHER STICKY SYSPUB ENABLED TYPE STATUS PROXY URI
solaris true true true origin online http://localhost:1008
http://ipkg.us.oracle.com/solaris11/dev/
extra true true true origin online http://localhost:1008
http://ipkg.us.oracle.com/opensolaris/extra/
internal-only true true true origin online http://localhost:1008
http://ipkg.us.oracle.com/internal/solaris11/internal-only/
solarisstudio true true true origin online http://localhost:1008
http://ipkg.us.oracle.com/solarisstudio/support/
on-extra true true true origin online http://localhost:1008
http://ipkg.us.oracle.com/internal/solaris11/on/extra/
userland true true true origin online -
http://localhost:1008/userland/614bdd284706b4be9743fa5021c76a968192c904/
and just for reference, what I was originally proposing:
# pkg -R /space/image publisher
PUBLISHER TYPE STATUS URI
solaris (syspub) origin online <system-repository>
extra (syspub) origin online <system-repository>
internal-only (syspub) origin online <system-repository>
solarisstudio (syspub) origin online <system-repository>
on-extra (syspub) origin online <system-repository>
userland (syspub) origin online
http://localhost:1008/userland/614bdd284706b4be9743fa5021c76a968192c904/
While I'm very enthusiastic about the change to the<system-repository>
display, I'm a little concerned about it taking place now, as opposed to
across a larger change boundary.
.
.
I'm not sure it needs to wait for a major release. It was more that I
wanted to ask the question because I'm still learning how to judge these
things, and hoped to get further comment on it.
Yep, I understand - does anyone else have an opinion here?
client.py:
I noticed changes only on set-publisher, do we need to allow --proxy on
commands which can take a temporary repository? ('pkg list -g<mumble>
--proxy' for example?)
Good point - I forgot that. There's an argument to be made that a
$http_proxy environment variable works just as well for something
that's supposed to be temporary in the first place, but I can add this
support if others feel it's worthwhile (it's a relatively large change
though, so would like to get consensus on whether we really need this
first)
I think I'd be fine with a filed RFE to do this in the future. I don't
think it has to go back as part of this wad, but I do think it's
something we should plan to have eventually.
Sure, I'll file the RFE. As is, the code makes sure that $http_proxy
environment variables will always override whatever --proxy values are
set in persistent publishers: for temporary publishers, that mechanism
is also used.
[snip]
596-627: I'm having a little trouble understanding why, given the new
info stored in lines 596-619, why we also need to store the origins and
mirrors separately?
Good question. I'm trying to be backwards compatible here, to allow
older clients to still read these images. Yes, there's a risk that
older pkg(5) bits could remove one of those origins, causing the
origin_info map to become stale, but I felt that was better than
causing an image-format bump.
Ok. Can you document it, and maybe at least hand test what happens if
the origin_info map becomes stale? Does it cause tracebacks, do we even
notice it at all, etc...
Yes, I'll do that.
[snip]
line 325: given how url_affix_trailing_slash is implemented, I don't get
why this line is necessary
It's necessary because it copes with input urls such as
http://foo/bar//
ensuring that we canonicalize them properly to simply
htt://foo/bar/
Ah. Hrm. I wonder whether that logic really belongs in
url_affix_trailing_slash, but I don't feel strongly about it.
Sure. I think I'd rather make that change when revisiting trailing-slash
handing in general. I'll file an RFE here too.
.
.
Ok, i think I follow now. Maybe a comment explaining why checking for ""
in self.allowed is a check for whether an empty list is allowed would be
helpful.
Good idea, I'll add that.
I won't post another webrev just yet, given we're undecided about the
"pkg publisher" output changes. To summarise, we need to decide:
* whether to show a PROXY column in the default "pkg publisher" output
* how to display system-repository-provided URIs
* whether we're happy with "-F tsv" output differing from the default
output
(my votes are as before:
* omit the proxy column from the default output in the GZ
* omit the proxy column from the NGZ, printing "<system repository>"
in place of the actual URI
* allow "-F tsv" to differ from default output for zones, printing
the actual URI, rather than the cosmetic "<system repository>"
string. )
cheers,
tim
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
