Danek Duvall wrote:
Shawn Walker wrote:
Danek Duvall wrote:
Shawn Walker wrote:
http://cr.opensolaris.org/~swalker/pkg-cat-p3/
modules/catalog.py:
...
- line 633: Is there any way you can think of to use "key" instead of
"cmp"?
Concatenating the stem and publisher (e.g. "%s!%s" % (stem, pub)) then
doing the sort, and then splitting them apart again as I iterate over the
sorted list?
That occurred to me, but I rejected it because I think I got the sense of
the test on 628 backwards. I agree that it should work. And it's supposed
to be quite a bit faster, though as K has shown us, testing it to be sure
would be a great idea. Doesn't have to be this wad.
It appears to work, and be faster. I imagine the overhead of using cmp=
is to blame here.
Over a million executions of the different tuples() methods for two
publishers with 400 unique FMRIs, the timing results were 207 seconds
for using a list comprehension and 219 seconds for the old iteration method.
- line 2201: why is this an exceptional condition, rather than just a
return of an empty list, as on line 2190?
Line 2190 is wrong and is a holdover from when the catalog.attrs file
was being conditionally retrieved. I've changed this.
Okay. Though part of my question is about the interface -- it doesn't seem
like an unmodified catalog is an exceptional condition, and that it really
*ought* to be returning something instead.
There's are subtle differences here that the exception is attempting to
differentiate between.
In one case, nothing has changed at all, not even catalog.attrs, and so
the client should just mark the publisher as having had a refresh and
then return.
In the second case, some part of the catalog that the client hasn't
retrieved yet (either because it doesn't use it, or because it's
locale-specific data that isn't applicable) has changed, and so the
catalog.attrs file has changed and thus apply_updates() still needs to
happen.
However, instead of raising an exception here, I can just return None
for the first case and continue to have the latter case return a [].
server/repository.py:
...
- line 770: this isn't safe; why not just copy old_cat_root into
tmp_cat_root? Or copy the contents of old_cat_root into tmp_cat_root?
Isn't safe in what manner?
There's a small window, between the rmtree and the copytree, where, because
the temporary directory you created doesn't exist, someone else could
perfectly sanely create another temporary directory with the same name.
I had assumed the risk was minimal since the mkdtemp is restricted to
the writable_root or repo_root as opposed to being in $TMPDIR.
It's hardly a big deal, though -- I just mentioned it in case there was an
easy way to not require the rmtree(), which would involve either copying
old_cat_root wholesale inside of tmp_cat_root, which would probably be
okay, though you'd need to do some massaging later on, or to copy the
contents of old_cat_root inside of tmp_cat_root, which would, I think,
require an os.listdir(), though not a full walk.
I'll probably use os.listdir() to walk the entries, use
portable.copyfile() on the files and use shutil.copytree on the
directories. At that point, nothing else special should be required.
extra_privs:
- is this change really part of this wad?
Hmm, this got picked up in a merge somehow. I don't know why it
thinks I added this file, since that's clearly part of changeset
1393. Looking at the Mercurial log though, it thinks I added it as
well. How to fix?
hg rm, I think.
An rm and a recommit mysteriously fixed it. Now it doesn't show me as
changing the file anymore.
Thanks,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
