PR#51210: Issue 51136 - JSON Error output has redundant messages - 389-ds-base

389-ds-base

#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

Issue 51136 - JSON Error output has redundant messages

Simon Pichugin • 2 years ago

632a8e0

src/cockpit/389-console/src/ds.jsx

file modified

+58 -45

		`@@ -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);`
		`}`

src/lib389/cli/dsconf

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`

src/lib389/cli/dscreate

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`

src/lib389/cli/dsctl

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`

src/lib389/cli/dsidm

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`

src/lib389/lib389/__init__.py

file modified

+2 -6

		`@@ -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`

spichugi commented 2 years ago

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: ?

mreynolds commented 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.

rebased onto 632a8e0

2 years ago

spichugi commented 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.

mreynolds commented 2 years ago

Tested and approved, ack!

Pull-Request has been merged by spichugi

2 years ago

spichugi commented 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.