I think it is simple and good enough for a minimal representation.

Ideally, I think, we better have a 'human-readable" format option for both UI and CLI.

What do you think?

Like we can add a setting for choosing units. Or just make a switch for representing in Mb (and for CLI it will be a true-false option)

I think it is simple and good enough for a minimal representation.
Ideally, I think, we better have a 'human-readable" format option for both UI and CLI.
What do you think?

The UI is human readable, how is it not? Now, the CLI could definitely be more readable

Like we can add a setting for choosing units. Or just make a switch for representing in Mb (and for CLI it will be a true-false option)

Yeah I can do that

@spichugi So to fix this I always convert bytes to a more friendly value, and I improved the CLI human readable format. Please review...

Minor thing - loadDiskSpace and reloadDisks have nearly the same code (I think only the log message is different). I think it would be better while encasulated...

The rest looks good. Ack.

Minor thing - loadDiskSpace and reloadDisks have nearly the same code (I think only the log message is different). I think it would be better while encasulated...
The rest looks good. Ack.

Sorry not sure what you mean. The big difference between the load and reload functions is that the load function calls other loading functions once its finishes setting the state. This is how it's done throughout the rest of the UI code.

I wonder if we can do something like this:

loadDiskSpace(firstLoad) {
    let cmd = [
        "dsconf", "-j", "ldapi://%2fvar%2frun%2fslapd-" + this.props.serverId + ".socket",
        "monitor", "disk"
    ];
    log_cmd("loadDiskSpace", "Load disk space info", cmd);
    cockpit
            .spawn(cmd, { superuser: true, err: "message" })
            .done(content => {
                let disks = JSON.parse(content);
                for (let disk of disks.items) {
                    disk.used = disk.used + " (" + disk.percent + "%)";
                }
                this.setState({
                    disks: disks.items
                });
                if (firstLoad) {
                    this.loadMonitorLDBM());
                }
             }
}
loadDisks() {
    this.loadDiskSpace(true);
}
reloadDisks() {
    this.loadDiskSpace(false);
}

I think it should work even if we put the this.loadMonitorLDBM()); callback inside of the callback from done().

That won't work correctly. The nested loading function must be "within" the setState call:

this.setState({
....
}, this.loadNextFunc);

I think it's best to leave the functions as separate functions. IMHO having similar code in two functions (that could be merged into a single function) is not as bad as it's made out to be. I feel ideology of avoiding duplicate code at all costs is antiquated. There are benefits to it, especially if the needs of the reload verses nested loading changes in the future. I feel it's more flexible and easier to read than adding a lot of wrappers and if/then/else conditions.

If you feel that strongly about it I will change it, but it also means we have to rewrite a lot of other code to be consistent with that model.

Okay, I got you. I think it makes sense to leave it like this here.

I think it's worth to have it in other places though... Like when we modify/add some modal window. It may have a lot of boilerplate react code and it can be nicely encapsulated for these kinds of similar functions.

You have my ack here! Thanks for the explanation!

Okay, I got you. I think it makes sense to leave it like this here.
I think it's worth to have it in other places though... Like when we modify/add some modal window. It may have a lot of boilerplate react code and it can be nicely encapsulated for these kinds of similar functions.

Please file an issue to address these areas.

You have my ack here! Thanks for the explanation!

Thanks!

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

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.