Bug 974893
Summary: | by_name() methods should convert NoResultFound to something more meaningful | ||
---|---|---|---|
Product: | [Retired] Beaker | Reporter: | Nick Coghlan <ncoghlan> |
Component: | web UI | Assignee: | beaker-dev-list |
Status: | CLOSED WONTFIX | QA Contact: | tools-bugs <tools-bugs> |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | 0.12 | CC: | jingwang, mastyk, mjia, qwan, tools-bugs |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2020-06-02 11:35:03 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 978225 | ||
Bug Blocks: |
Description
Nick Coghlan
2013-06-17 04:09:53 UTC
Using an appropriate custom exception should also make it possible to handle these in a relatively generic fashion in the web UI layer (converting them to a 200 OK response with a suitable error message rather than 500 error). Is there any reason why any lookup method (i.e by_id()) not be included in this? I happened to notice lots of "by_name" instances in the logs, but yes, it would probably be applicable to other lookup mechanisms as well (since we do expose lookups by ID in a few places) We catch the NoResultException in some of these queries and raise a ValueError with a friendly error message. Is the proposed fix run *all* such queries in a try..catch block and have it raise a custom exception with a friendly message? For eg. try: SomeClass.by_name('something') except NoResultFound: raise MyCustomClass('Something bad happened') (In reply to Amit Saha from comment #4) > We catch the NoResultException in some of these queries and raise a > ValueError with a friendly error message. > > Is the proposed fix run *all* such queries in a try..catch block and have it > raise a custom exception with a friendly message? > > For eg. > > try: > SomeClass.by_name('something') > except NoResultFound: > raise MyCustomClass('Something bad happened') Yes except it would be try: return cls.query.filter(cls.name == name).one() except NoResultFound: raise DatabaseLookupError('No such blah %r' % name) That is, the try-except-raise should be in the by_name method itself (and this applies to all the other by_* lookup methods, in spite of the bug title). Ray implemented this in one of his patches a while back but it got bogged down in deciding what the exception type and exception message should be, so we abandoned it. I think for the exception type ,we could borrow some ideas from openstack. http://docs.openstack.org/developer/python-openstackclient/api/openstackclient.common.exceptions.html Cheers, Matt Jia If I give a non-existent pool name in cli, it will return "No row was found for one()" See below. [wangdong@localhost tmp]$ bkr system-modify beaker-test-vm1 --pool-policy PoolBaa /usr/lib/python2.7/site-packages/requests/packages/urllib3/connection.py:251: SecurityWarning: Certificate has no `subjectAltName`, falling back to check for a `commonName` for now. This feature is being removed by major browsers and deprecated by RFC 2818. (See https://github.com/shazow/urllib3/issues/497 for details.) SecurityWarning HTTP error: 400 Client Error: BAD REQUEST No row was found for one() I think it should tell user "There isn't a pool named <PoolBaa>" Hello, thank you for opening issue in Beaker project. This issue was marked with component "web ui". As we are not planning to address any further issues in current UI, due to technical stack and not being able to work with Python 3 codebase, I'm closing this issue as WONTFIX. New UI will be reimplemented within new versions of Beaker. If you have any questions feel free to reach out to me. Best regards, Martin <martin.styk> |