#50248 Ticket 50230 - improve ioerror msg when not root/dirsrv
Closed 3 years ago by spichugi. Opened 5 years ago by firstyear.
firstyear/389-ds-base 50230-ioerror-no-permission  into  master

file modified
+8 -3
@@ -103,11 +103,16 @@ 

          signal.signal(signal.SIGINT, signal_handler)

          try:

              insts = inst.list(serverid=args.instance)

-         except PermissionError:

-             log.error("Unable to access instance information. Are you running as root or dirsrv?")

+         except (PermissionError, IOError) as e:

+             log.error("Unable to access instance information. Are you running as the correct user? (usually dirsrv or root)")

+             log.error("Error: %s" % str(e))

+             sys.exit(1)

+         except Exception as e:

+             log.error("Error: %s" % str(e))

              sys.exit(1)

      if len(insts) != 1:

-         log.error("No such instance '%s': this may be a permission issue." % args.instance)

+         log.error("No such instance '%s'" % args.instance)

+         log.error("Unable to access instance information. Are you running as the correct user? (usually dirsrv or root)")

          sys.exit(1)

  

      inst.allocate(insts[0])

Bug Description: When not running as root or dirsrv, improve the clarity
of the error messages as the previous messages were misleading.

Fix Description: Improve the exception handling and messages.

https://pagure.io/389-ds-base/issue/50230

Author: William Brown william@blackhats.net.au

Review by: ???

You can do except (PermissionError, IOError) as e: rather than duplicating the code.

Unless I'm mistaken. we can run as different users as well. It could be rather confusing. Can we have this error dynamic or not mention user names at all? (same below, of course)

@mhonek That's a good tip to prevent the duplication.

We can run as different users, but it's really unlikely (unless in containers). The thing is though, that if we are running "not as root/dirsrv" and the operation works ... then we are running as the right use. If we aren't root/dirsrv and it fails, it probably is wrong .... It's a really tricky balance here because if we say nothing abotu the user, we are missing good hints to fix the problem. If we say too much, we then risk confusion. I think that mentioning this is reasonable to me since they are the two most common users related to admin on rhel/fedora/suse.

rebased onto ee0dcf07a3e1933a94b9528302dcaf636ac85d9e

5 years ago

Fair enough. Only, I would change wording then to something like ... Are you running as correct user (usually root or dirsrv)?

Besides that, ACK from me. Thanks.

OHHH I think you didn't communicate that intent very well ... perhaps an example message next time. Hahha. That's actually a reasonable improvement. I understood it as you trying to remove all references to dirsrv/root.

rebased onto 6400df10618bd40b6ae96dc6cd787e1530280d0c

5 years ago

:thumbsup: Merge please.

rebased onto f166154

5 years ago

Pull-Request has been merged by firstyear

5 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3307

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago
Metadata