ceph.in: do not throw on unknown errno #4375
Conversation
tchaikov
force-pushed
the
wip-ceph-unrecognized-exception
branch
from
bdfaa2e
to
9d1b8a6
Compare
|
|
||
| # just a couple of globals | ||
|
|
||
| # in case some errno is not recognized. | ||
| errorcode = defaultdict(lambda: 'N/A') |
There was a problem hiding this comment.
Perhaps "Unknown" would be a more literal statement here? N/A to me sounds like we're saying an error doesn't matter.
There was a problem hiding this comment.
thanks for your review, @jcsp updated according to your suggestion.
tchaikov
force-pushed
the
wip-ceph-unrecognized-exception
branch
2 times, most recently
from
989962f
to
9e9b3a3
Compare
|
it looks good, will test is manually. Out of curiosity: why not use errno.errorcode.get(ret, 'Unknown') ? |
|
@dachary , oh, i just recalled that i wanted to minimize the code churn introduced by this change. and there are three places we are using |
|
@tchaikov ok. How do you suggest I test that manually ? I tried $ ./ceph
*** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH ***
Error initializing cluster client: Error('error calling conf_read_file: errno EINVAL',)
but creating the conditions for the 'Unknown' errno to show seems a little tricky. |
|
@dachary yeah, it's tricky. how about
|
|
@tchaikov of course ! |
|
@tchaikov I manually set a wrong ret code in the code and saw ceph> auth list Error: 230948 Unknown |
|
ceph.in is quite difficult to test, even manually :-) |
ghost
self-assigned this
|
true. actually, i was testing by whipping up a wrapper around thanks for the review and testing =) |
|
@tchaikov just to be sure there is no misunderstanding: you're going to amend the current patch ? |
tchaikov
force-pushed
the
wip-ceph-unrecognized-exception
branch
from
9e9b3a3
to
16f8a4f
Compare
|
@dachary just updated this patch. thought that defaultdict could avoid repeating "Unknown", but seems |
some of the errnos are not listed in errno.errorcode. if we happen to run into them, print 'Unknown' instead. Fixes: ceph#11354 Signed-off-by: Kefu Chai <kchai@redhat.com>
tchaikov
force-pushed
the
wip-ceph-unrecognized-exception
branch
from
16f8a4f
to
53514ad
Compare
|
@dachary fixed. could you take a look again? thanks. |
|
as soon as the bot comes back green Reviewed-by: Loic Dachary <ldachary@redhat.com> |
ceph.in: do not throw on unknown errno Reviewed-by: Loic Dachary <ldachary@redhat.com>
ghost
merged commit 
some of the errnos are not listed in errno.errorcode, if we happen
to run into them print 'Unknown' instead.
Fixes: #11354
Signed-off-by: Kefu Chai kchai@redhat.com