Hi,
[merged the comments]
> > - yield url[:-len("/repodata/repomd.xml")]
> > + yield url[:-len("/repodata/repomd.xml")],
> > mirror.max_connections
>
> We can't change the API of metalink.urls like this. This is one of the
> reasons we still aren't using the private mirror attribute. Probably
> need some new API to return "url objects" which will have
> private/max_connections/etc. attributes.
I've changed metalink.urls() so it returns URLs, but also stores
max_connections in metalink.host2con hash. Have to use hostnames
because yumRepo does some further URL rewriting.
> ...so if we are going to add random attributes to the dict we should
> document that ... or just use the kwargs indirection, and document
> that.
Documented kwargs['max_connections'].
> however "failfunc" seems pretty bad as a name.
Can't think of better one ATM. It's ment to be 'dual' to the check-func,
with similar semantics (being called only once).
> Also having two code paths seems non-optimal, can't we have the
> default "failfunc" just do the raise?
Did that. Also changed the MirrorGroup => Grabber argument passing
quite a bit, so that blocking grabber does not receive 'failfunc',
and hence does not have to check for the MG layer.
> Finally the _callback() method could use a better name ... I guess
> it's "_run_callback" or "_call_callable" ... which still aren't good, but
> give some idea.
I think _run_callback is fine, renamed that.
> Why can't we reuse _curl_cache ... what problems does that cause?
> IIRC keepalive doesn't work if you do that.
Uh, thanks for spotting this. It should be possible to reuse them,
will implement that.
[wrt CurlMulti]
> Again, _if_ we want to use this then it should be an optimisation.
> I've had _so many problems_ with using non-multi curl talking to multiple
> hosts that I have no expectation that using multi. curl will make
> anything better/easier/whatever.
Multi curl means just less forking and simpler polling, but I
think of implementing a pipe-for-each-request version too-
to have 'plan B' if we hit a wall with multi.
> > +class _ProxyProgress:
> > + def start(*d1, **d2): pass
> > + def update(self, _amount_read):
> > + os.write(1, '%d %d\n' % (self._id, _amount_read))
>
> Don't you need "end" here too?
CurlFileObject calls start + update during the download.
"end" (or "failure") method is called by the upper layer.
_do_fo_close() does not call "end" either.
CurlFileObject's close() DOES call "end", but it's avoided,
because (contrary to _do_grab), _do_fo_close does not reopen it.
> > +import simplejson
>
> Is this really necessary ... how big is the cost?
Don't like it either.. I have used a simple value formatting
code for int/float/str options only, but 'range' option needs
a tuple, so instead of 'if option_name == ...', I gave up.
$ python -c 'print open("/proc/self/status").read()'|grep RSS
VmRSS: 3092 kB
$ python -c 'import simplejson;simplejson.dumps(None);print
open("/proc/self/status").read()'|grep RSS
VmRSS: 4340 kB
1.2M, expected it to be much less.
> > + fdset = dl.multi.fdset()
> > + fdset[0].append(0)
> > + if 0 in select.select(*(fdset + (tout,)))[0]:
>
> Again, select.poll() code is going to be 666 times easier to read.
I'd have to use some fdset() => poll.register() adapter..
Not sure if that's easier to read than these 3 lines.
> ... basically a blocking readline() call, which we can probably
> live with (although it can suck). But at least put it behind some
> method.
Ok.
> > + # Do some polling to work this around.
> > + tout = 10e-3
> Shocker, workarounds for CurlMulti weirdness.
Yes, it sucks. I guess it's because of the way curl does dns- that socket is
probably burried somewhere too deep to get into curl multi's fdset().
But it's just few iterations, does not hurt much, IMO.
> Is it a big benefit to use __file__ instead of creating something in
> libexec/whatever?
Not sure. What are the options? Hard-coded paths? Config options?
I don't even like that '/usr/bin/python' above..
> Have you tried on RHEL-6 with ssl cert. repos?
No, just the very basic use cases.
Zdenek
_______________________________________________
Yum-devel mailing list
Yum-devel@lists.baseurl.org
http://lists.baseurl.org/mailman/listinfo/yum-devel
