[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e
- From: Erik Trauschke <
- To: Yiteng Zhang <
- Subject: [pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e
- Date: Tue, 29 Oct 2013 13:22:20 -0700
On 10/25/13 01:31 PM, Yiteng Zhang wrote:
Ok, a new webrev can be seen at
Sorry that I dont add a comment to the new webrev.
Please let me know your comments.
Looking at this it seems a cleaner approach would be to collect the
exceptions in a simple list first and then only raise the exception if
anything is in this list. Look at how InvalidOptionErrors is used,
basically you can do the same thing here.
This way you don't need an additional variable to keep track if you
actually had at least one exception and you only create the
ExpiredCertificates exception when needed. That's a small change.
relying on the fact that publishers are always in the correct sequence
is sketchy. It's probably better to create a dictionary (publisher:
[uri1, uri2, ...]) and then cycle through all the publishers in this list.
There is some work needed here to have these messages properly
localized. The problem is that the length of the fields is not fixed in
a way that we can format this message appropriately since the field
names might have different lengths in different languages. The
localization folks don't want to have any space formatting in the
message strings for that reason so I suggest we do the following:
You should be able to generate this like that:
msg = "%s\n" % publisher
msg += " %s:\n" % _("Origin URI")
msg += " %s\n" % uri
Please break the lines like this: