#51210 Issue 51136 - JSON Error output has redundant messages
Closed 2 years ago by spichugi. Opened 2 years ago by spichugi.
spichugi/389-ds-base i51136  into  master

@@ -59,7 +59,7 @@ 

              backupRows: [],

              notifications: [],

              activeKey: 1,

-             wasActiveList: [1],

+             wasActiveList: [],

              progressValue: 0,

              loadingOperate: false,

  
@@ -137,7 +137,8 @@ 

                                      this.updateProgress(25);

                                      this.setState(

                                          {

-                                             serverId: serverId

+                                             serverId: serverId,

+                                             wasActiveList: [this.state.activeKey]

                                          },

                                          () => {

                                              this.loadBackups();
@@ -146,11 +147,13 @@ 

                                      if (action === "restart") {

                                          this.setState(

                                              {

-                                                 serverId: ""

+                                                 serverId: "",

+                                                 wasActiveList: []

                                              },

                                              () => {

                                                  this.setState({

-                                                     serverId: serverId

+                                                     serverId: serverId,

+                                                     wasActiveList: [this.state.activeKey]

                                                  });

                                              }

                                          );
@@ -168,7 +171,8 @@ 

                                          },

                                          () => {

                                              this.setState({

-                                                 serverId: serverId

+                                                 serverId: serverId,

+                                                 wasActiveList: []

                                              });

                                          }

                                      );
@@ -183,7 +187,8 @@ 

                              },

                              () => {

                                  this.setState({

-                                     serverId: serverId

+                                     serverId: serverId,

+                                     wasActiveList: []

                                  });

                              }

                          );
@@ -201,7 +206,8 @@ 

                          },

                          () => {

                              this.setState({

-                                 serverId: serverId

+                                 serverId: serverId,

+                                 wasActiveList: []

                              });

                          }

                      );
@@ -217,51 +223,59 @@ 

                  }

              }));

          }

-         let cmd = ["dsctl", "-l", "-j"];

-         log_cmd("loadInstanceList", "Load the instance list select", cmd);

-         cockpit

-                 .spawn(cmd, { superuser: true })

-                 .done(data => {

-                     this.updateProgress(25);

-                     let myObject = JSON.parse(data);

-                     this.setState({

-                         instList: myObject.insts,

-                         loadingOperate: false

-                     });

-                     // Set default value for the inst select

-                     if (serverId !== undefined && serverId !== "") {

-                         this.setState({

-                             wasActiveList: [this.state.activeKey]

-                         });

-                         this.setServerId(serverId, action);

-                     } else {

-                         if (myObject.insts.length > 0) {

+         this.setState(

+             {

+                 wasActiveList: []

+             },

+             () => {

+                 let cmd = ["dsctl", "-l", "-j"];

+                 log_cmd(

+                     "loadInstanceList",

+                     "Load the instance list select",

+                     cmd

+                 );

+                 cockpit

+                         .spawn(cmd, { superuser: true })

+                         .done(data => {

+                             this.updateProgress(25);

+                             let myObject = JSON.parse(data);

                              this.setState({

-                                 wasActiveList: [this.state.activeKey]

+                                 instList: myObject.insts,

+                                 loadingOperate: false

                              });

-                             this.setServerId(myObject.insts[0].replace("slapd-", ""), action);

-                         } else {

+                             // Set default value for the inst select

+                             if (serverId !== undefined && serverId !== "") {

+                                 this.setServerId(serverId, action);

+                             } else {

+                                 if (myObject.insts.length > 0) {

+                                     this.setServerId(

+                                         myObject.insts[0].replace("slapd-", ""),

+                                         action

+                                     );

+                                 } else {

+                                     this.setState({

+                                         serverId: "",

+                                         pageLoadingState: {

+                                             state: "noInsts",

+                                             jsx: staticStates["noInsts"]

+                                         }

+                                     });

+                                 }

+                             }

+                         })

+                         .fail(_ => {

                              this.setState({

+                                 instList: [],

                                  serverId: "",

+                                 loadingOperate: false,

                                  pageLoadingState: {

                                      state: "noInsts",

                                      jsx: staticStates["noInsts"]

                                  }

                              });

-                         }

-                     }

-                 })

-                 .fail(_ => {

-                     this.setState({

-                         instList: [],

-                         serverId: "",

-                         loadingOperate: false,

-                         pageLoadingState: {

-                             state: "noInsts",

-                             jsx: staticStates["noInsts"]

-                         }

-                     });

-                 });

+                         });

+             }

+         );

      }

  

      loadBackups() {
@@ -319,8 +333,7 @@ 

      handleServerIdChange(e) {

          this.setState({

              pageLoadingState: { state: "loading", jsx: "" },

-             progressValue: 25,

-             serverId: e.target.value

+             progressValue: 25

          });

          this.loadInstanceList(e.target.value);

      }

file modified
+1 -1
@@ -139,7 +139,7 @@ 

          msg = format_error_to_dict(e)

  

          if args and args.json:

-             sys.stderr.write(json.dumps(msg, indent=4))

+             sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")

          else:

              log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))

          result = False

file modified
+1 -1
@@ -80,7 +80,7 @@ 

          log.debug(e, exc_info=True)

          msg = format_error_to_dict(e)

          if args and args.json:

-             sys.stderr.write(json.dumps(msg, indent=4))

+             sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")

          else:

              log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))

          result = False

file modified
+1 -1
@@ -141,7 +141,7 @@ 

          log.debug(e, exc_info=True)

          msg = format_error_to_dict(e)

          if args.json:

-             sys.stderr.write(json.dumps(msg, indent=4))

+             sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")

          else:

              log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))

          result = False

file modified
+1 -1
@@ -134,7 +134,7 @@ 

          log.debug(e, exc_info=True)

          msg = format_error_to_dict(e)

          if args.json:

-             sys.stderr.write(json.dumps(msg, indent=4))

+             sys.stderr.write(f"{json.dumps(msg, indent=4)}\n")

          else:

              log.error("Error: %s" % " - ".join(str(val) for val in msg.values()))

          result = False

@@ -1130,9 +1130,7 @@ 

          if self.with_systemd():

              self.log.debug("systemd status -> True")

              # Do systemd things here ...

-             subprocess.check_call(["systemctl",

-                                    "start",

-                                    "dirsrv@%s" % self.serverid])

+             subprocess.check_output(["systemctl", "start", "dirsrv@%s" % self.serverid], stderr=subprocess.STDOUT)

          else:

              self.log.debug("systemd status -> False")

              # Start the process.
@@ -1201,9 +1199,7 @@ 

          if self.with_systemd():

              self.log.debug("systemd status -> True")

              # Do systemd things here ...

-             subprocess.check_call(["systemctl",

-                                    "stop",

-                                    "dirsrv@%s" % self.serverid])

+             subprocess.check_output(["systemctl", "stop", "dirsrv@%s" % self.serverid], stderr=subprocess.STDOUT)

          else:

              self.log.debug("systemd status -> False")

              # TODO: Make the pid path in the files things

Bug Description: When we try to start an instance for which
'systemctl start' command has failed, it produces excessive
output which is not a clear JSON.

Fix Description: Redirect stderr to stdout as we don't need
the info in CLI. User needs to check logs if something went wrong.
Also, add a new-line character in the end of DS CLI tool's stderr.

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

Reviewed by: ?

I'm worried that you adding a newline might break the UI when it tries to parse the JSON object. Did you check if the UI correctly reports the errors now? If does work in the UI then you get my ack.

rebased onto 632a8e0

2 years ago

I'm worried that you adding a newline might break the UI when it tries to parse the JSON object. Did you check if the UI correctly reports the errors now? If does work in the UI then you get my ack.

It works fine.

I've also fixed the issue that was printing a lot of messages (or crashing) when we switch to another instance that was not online.

Please, review the React code cleanup.

Tested and approved, ack!

Pull-Request has been merged by spichugi

2 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/4263

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

2 years ago