I'm thinking about extending convertFault a small bit. If faultCode is
present it should almost always come from koji. In such a case I would say
it is +- safe to convert it to GenericError instead with the following
code. There are probably some extreme cases where it could be misleading
(faultCode is not from koji but from the underlying layer of xmlrpclib),
but it should be safe in the most cases. Another improvement could be that
faultCode range could be part of e.g. getKojiVersion(extended=True) which
could return dict with version itself, coderange and maybe some additional
info (like plugins enabled at hub).
About the importance - I think that the biggest problem wouldn't be the CLI
- it is easy to see that it failed, but automation scripts and possible
plugins. Their current GenericError handlers will stop working and just
propagate exceptions further. So, wrapping everything as GenericError seems
like a safer way to me.
Another options is the original one: Introduce new Exception ASAP but start
using them in a ~year. I don't think it is that much helping. I see that
(client) deployments are mostly in two categories - they don't update
almost at all (old isolated boxes) and those which are updating for almost
every release. It would help us in those small time windows (fedora
upgraded koji hub, but users still don't have actual systems). It is still
worth, but it can be reduced to +1 release (introduce exception in 1.27 and
start them using in 1.28).
Mike, any thoughts?
for v in globals().values():
if isinstance(v, type(Exception)) and issubclass(v, GenericError)
and \
code == getattr(v, 'faultCode', None):
ret = v(fault.faultString)
ret.fromFault = True
return ret
+ else:
+ raise GenericError(fault.faultString)
On Thu, Oct 21, 2021 at 6:17 PM Ken Dreyer <ktdreyer(a)ktdreyer.com> wrote:
On Thu, Oct 21, 2021 at 7:20 AM Tomas Kopecek
<tkopecek(a)redhat.com> wrote:
>
> I'm +1 here. I'm wondering (and we've had that discussion already
> somewhere) why we should wait with introducing new exceptions. If we
> just change GenericErrors to new ones we shouldn't break anything
> (older code will still catch GenericError descendants). And I think
> there was some good reason, Mike?
I actually tested this recently, raising a new custom exception in
get_user(). My notes are in
https://github.com/ktdreyer/koji-ansible/issues/221
The biggest general problem that I can see is that old clients's
convertFault() method does not translate new (unknown) faults to a
GenericError. It simply returns the xmlrpc.client.Fault as-is. In some
cases, old clients have code like:
try:
session.someRPC()
except GenericError:
# nice error handling here
Rather than catching all possible faults like:
try:
session.someRPC()
except (GenericError, six.moves.xmlrpc_client.Fault):
# nice error handling here
Both patterns are present in koji's codebase. But the former pattern
could lead to newly un-handled exceptions.
It's not clear to me how important this is. I haven't looked at every
bit of code that catches GenericError yet. I *am* finding that a lot
of CLI commands simply make RPCs without catching anything at all, so
they just let any exceptions bubble up. For example, like I mentioned
in that koji-ansible issue, the `koji set-pkg-owner` CLI has no
try/except at all, and as a result, the UX does not differ very much.
The error text looks a tiny bit different, but as long as we maintain
a faultString on the hub that starts with "No such user", I think a
human can still understand what Koji is saying.
- Ken
_______________________________________________
koji-devel mailing list -- koji-devel(a)lists.fedorahosted.org
To unsubscribe send an email to koji-devel-leave(a)lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/koji-devel@lists.fedorahoste...
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
--
Tomas Kopecek <tkopecek(a)redhat.com>
RHEL Build Development, RedHat