389-ds-base

#49776 Ticket 49712 - lib389 CLI tools should return a result code on failures
Closed by spichugi. Opened by mreynolds.
mreynolds/389-ds-base ticket49775  into  master

Download 49776.patch

Description: I've also included the work for 49775 in this patch since
there was a lot of overldap.

          For dsctl functions we need to check for True and False in
          order to detect an error.  For dsconf & dsidm we need to
          catch exceptions.  Once an error is detected we return error
          code (1).
          The changes for 49775 was to use the default archive directory
          if one was not specified to db2bak, and use the default ldif
          location for db2ldif.  This how the old tools worked, no
          reason not to carry over this convenience.  Also the format
          used for the file name (Instance name + Date/Time) is the same
          as the old cli tools.
          Also did some pep8 cleanup.

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

rebased onto e407dc250aca10d91ab9e415eddb45cd4a5c1777

If I understand correctly, by documentation, return_value = PR_Rename(directory, dir_bak); returns either PR_SUCCESS (0), either PR_FAILURE (-1). So the line you've added seems redundant to me... Or do I miss something?

I think we should print the error to debug so we don't lose the information.
You can rewrite the thing like this:

try:
    result = subprocess.check_output(cmd, stderr=subprocess.STDOUT, encoding='utf-8')
except subprocess.CalledProcessError as e:
    self.log.debug("Command: {} failed with the return code {} and the error {}".format(format_cmd_list(cmd), e.returncode, e.output))
    return False

Its MKDIR not PR_RENAME, and yes it is needed or else it silently fails.

Sure I can do that

rebased onto e6a061e93b44c2f46aea28b21aefce5f17d712a5

My bad... I've looked at the wrong line in https://pagure.io/fork/mreynolds/389-ds-base/blob/ticket49775/f/ldap/servers/slapd/back-ldbm/archive.c
You are right.

I've tested the change and it works. Beside the small thing we discussed (debugging output for other subprocess.check_output changes - the same way as the first one), LGTM! Ack.

rebased onto 5226bf99f18008cb956a9b2ae0dc5ea2a5fc4a7f

Pull-Request has been merged by mreynolds

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/2835

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
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.