[Yum-devel] yum-security plugin review, possible merge into yum-utils?

[James Antill]

On Mon, 2007-04-23 at 23:49 -0400, Luke Macken wrote:

> Nice work!
> 
> After a quick glance, here are a few things that come to mind:
> 
>     o ysp_check_func_enter() gets called from both the exclude_hook as
>       well as the postresolve_hook.  Would it be possible to simply call
>       it once and store 'skip'/'list_cmd' somewhere globally?

 Right, we could store the results from the exclude_hook call somewhere
for the preresolve_hook call. Although it's not really doing a lot of
work, so I thought it would be better to have the hooks look similar.

>     o ysp_gen_metadata() is called from both hooks as well, which means
>       that we parse the updateinfo.xml.gz.twice.  Why not just keep a
>       global UpdateMetadata object?

 This is not true, as ysp_check_func_enter() sets info["skip"] and if
this is true the rest of the exclude hook is run (but not the rest of
the preresolve_hook) and if it's false it's the other way around.
 See the huge comment just before ysp_should_keep_pkg(), for why there
are two hooks.

>     o ysp_show_pkg_md_info() -- This looks like it would best fit into
>       yum.update_md.UpdateNotice.__str__.  Right now the __str__ for
>       UpdateNotices is a bit ugly, but improvements are definitely
>       welcome, and that seems like the place to do something like this.

 Very much so, but I didn't want to just reassign parts of the class. If
you think it's good to change it though, feel free to replace the
__str__ function and then delete that function with a simple print.

> Ideas/Suggestions:
> 
>     o --{advisory,bz,cve} should be able to refine {list,info}-sec results

 Right, I had thought of this, and also merging the "list-sec bzs" in
the other direction ... but then it gets more crufty, so I did
neither :).
 I'll post a patch to do --bz and --cve for the list/info commands.

>     o list-sec should only list updates (id - nvr - title, or something),
>       instead of displaying details; while show-sec could simply just
>       `print notice` and allow the yum.update_md.UpdateNotice.__str__ to
>       do the rest.


 Right, I'd thought of trying to make list-sec even more compact. You'd
probably still want to know which are md['type'] == "security", but that
could just be a single char somewhere.

> I'm hoping to find some free time in the future to play around with this
> a bit more, but it's looking good so far.  I'm definitely in favor of
> getting this into yum-utils as well.

 I'ts already in the yum-utils CVS, I assume Tim Lauridsen merged it.

-- 
James Antill <jantill at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.baseurl.org/pipermail/yum-devel/attachments/20070424/c60b8c4c/attachment.pgp