Yuli,
If you respin this with just import error looks like its a go.
Bill

On Fri, Oct 5, 2018 at 12:53 PM Chris PeBenito <peben...@ieee.org> wrote:

> On 10/05/2018 10:32 AM, Jason Zaman wrote:
> > On Fri, Oct 05, 2018 at 07:13:23AM -0700, William Roberts wrote:
> >> On Thu, Oct 4, 2018 at 12:46 PM Yuli Khodorkovskiy <
> >> yuli.khodorkovs...@crunchydata.com> wrote:
> >>
> >>> The python module import error in semanage_migrate_store was
> misleading.
> >>> Before, it would print that the module is not installed, even though
> >>> it is in fact on the system.
> >>>
> >>> Now the python module import failure is correctly reported if the
> module
> >>> is not installed or the exact reason for failure is reported to the
> user.
> >>>
> >>> Signed-off-by: Yuli Khodorkovskiy <y...@crunchydata.com>
> >>> ---
> >>>   libsemanage/utils/semanage_migrate_store | 6 ++++--
> >>>   1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/libsemanage/utils/semanage_migrate_store
> >>> b/libsemanage/utils/semanage_migrate_store
> >>> index 2e6cb278..50eb59ef 100755
> >>> --- a/libsemanage/utils/semanage_migrate_store
> >>> +++ b/libsemanage/utils/semanage_migrate_store
> >>> @@ -15,10 +15,12 @@ sepol = ctypes.cdll.LoadLibrary('libsepol.so.1')
> >>>   try:
> >>>          import selinux
> >>>          import semanage
> >>> -except:
> >>> +except ImportError:
> >>>          print("You must install libselinux-python and
> libsemanage-python
> >>> before running this tool", file=sys.stderr)
> >>>          exit(1)
> >>> -
> >>> +except Exception as e:
> >>> +       print("Failed to import libselinux-python/libsemanage-python:
> %s"
> >>> % str(e))
> >>> +       exit(1)
> >>>
> >>
> >> We should really only be handling exceptions we reasonably expect and
> >> discourage
> >> the usage of catching raw Exception, especially considering not-catching
> >> this will
> >> cause the runtime to print a stack trace, the error and exit non-zero.
> >>
> >> We probably only need the except ImportError change and can drop the
> second
> >> hunk.
> >>
> >> Does anyone disagree with this?
>
> For this case, I agree that ImportError is the only thing that should be
> caught.
>
> > Agreed. catching Exception is bad cuz it also catches KeyboardInterrupt
> > and stuff like that.
>
> That's not correct.  Catching BaseException would catch
> KeyboardInterrupt.  Catching Exception would not.  See the Python
> builtin exception hierarchy:
>
> https://docs.python.org/3/library/exceptions.html#exception-hierarchy
>
> IMO catching Exception has valid uses.
>
> --
> Chris PeBenito
>
_______________________________________________
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Reply via email to