[#385][#390] JSON command output implementation for `pgagroal-cli` #391

fluca1978 · 2023-12-06T14:25:57Z

This commit introduces the capaiblity for pgagroal-cli to print out the command results in a JSON format.

A new dependency on the library 'cJSON' has been introduced; see https://github.com/DaveGamble/cJSON.

A new set of functions, named 'json_xxx' have been added in a separated file json.c (include file json.h) to handle JSON structures in a more consistent way.

Each function that manipulates a JSON object has been named with the 'json_' prefix. Each management function that reads data out of the protocol and creates or handles json data has been named with the prefix 'pgagroal_management_json_'.
A few functions with a prefix name 'pgagroal_management_json_print' are used to print out a JSON object as normal text.

Every command output, in the JSON format, is structured with a 'command' is the main object contained in the output, that in turns has different attributes:

'output' is an object that contains the command specific information (e.g., list of databases, messages, and so on);
error a boolean that indicates if the command was in error or not;
status a string that contains either 'OK' or an error message in the case the error flag is set;
exit-status an integer value with the exit status of the command, 0 in case of success, a different value in case of failure.

The JSON object for a result includes also another object, named 'application', that can be used for introspection: such object describes which executable has launched the command (so far, only pgagroal-cli) and at which version.

In the 'output' object, every thing that has a list (e.g., connections, limits, databases, servers) will be wrapped into an object with the attributes 'list' and 'count': the former contains the details of every "thing", while the latter contains the count (i.e., the size of the array 'list' ).
Whenever possible, a 'state' string is placed to indicate the state of the single entry.

The command status and status details have been unified on the management side.
There is a single function that handles both the cases of reading the answer for the status or the status details commands. This has been done because status details includes the output of status. The function pgagroal_management_json_read_status_details is in charge of getting the answer of the management protocol (i.e., read the socket) and invoke the printing function in the case the output is of type text. The above pgagroal_management_json_read_status_details returns always a JSON object, that can be converted back to the text output via pgagroal_management_json_print_status_details. In this way, the output between the JSON and the text formats are always coherent for these commands.

The ping (i.e., is-alive) command does not print nothing by default. In the JSON version it provides the numerical status of the server and a string that represents a human-readable status.

The conf get command has been refactored to be symmetric with other commands: the logic to print out the result is now within the management function (pgagroal_management_read_config_get) as per other commands. The JSON provides the config-key and the config-value as strings. See #390

The conf set command has been refactored similarly to conf get in order to have all the logic to print out the information into the management read method (see #390).
The exit status provided by the command is now
the result of the matching within the JSON object of the expected configuration value and the requested configuration value.

The conf ls command has been refactored to produce a JSON object when reading data out of the management socket. Such JSON object is then printed as normal text if required.

A new utility function, named 'pgagroal_server_status_as_string' has been added to the utils.c stuff. The idea is to have a consistent way to translate the numerical status representation into an human readable string.

The text output format of commands has slightly changed due to the refactoring of some internal methods.

Documentation updated.

Close #385
Close #390

jesperpedersen · 2023-12-07T08:15:44Z

You forgot to update https://github.com/agroal/pgagroal/blob/master/.github/workflows/ci.yml

fluca1978 · 2023-12-07T08:31:09Z

You forgot to update https://github.com/agroal/pgagroal/blob/master/.github/workflows/ci.yml

Or better, I'm not used to CI setup, so I'm learning in here! I've pushed 85ea9d6 that hopefully solves the problem on the linux side.

jesperpedersen · 2023-12-07T08:42:38Z

/home/runner/work/pgagroal/pgagroal/src/libpgagroal/json.c:167:4: error: implicit declaration of function ‘cJSON_SetBoolValue’; did you mean ‘cJSON_SetIntValue’? [-Werror=implicit-function-declaration]
  167 |    cJSON_SetBoolValue(current, true);
      |    ^~~~~~~~~~~~~~~~~~
      |    cJSON_SetIntValue

fluca1978 · 2023-12-07T08:44:25Z

Gosh!
I tested the patch against cJSON 1.7.16 that has the cJSON_SetBoolValue function (see https://github.com/DaveGamble/cJSON/releases), but apparently ubuntu is using cJSON 1.7.15 (see https://github.com/agroal/pgagroal/actions/runs/7125616691/job/19401885600#step:5:43) and thus it is failing (https://github.com/agroal/pgagroal/actions/runs/7125616691/job/19401885600#step:11:9).

I would not rewrite the parsing using an int instead of a boolean value, it is not that difficult, but I think it is wrong in logic. How can we fix this?

jesperpedersen · 2023-12-07T08:44:52Z

CMake Warning at CMakeLists.txt:83 (find_package):
  By not providing "FindcJSON.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "cJSON", but
  CMake did not find one.

There is one here - https://github.com/pgmoneta/pgmoneta/blob/main/cmake/FindCJSON.cmake

jesperpedersen · 2023-12-07T08:52:05Z

Fedora is using 1.7.14.

Having cJSON_SetBoolValue in a 1.7.16 release sounds wrong to me - it should be 1.8.0 instead since the API has additions.

I think we need an int here with 0 | 1 as values

fluca1978 · 2023-12-07T12:03:40Z

Fedora is using 1.7.14.

Having cJSON_SetBoolValue in a 1.7.16 release sounds wrong to me - it should be 1.8.0 instead since the API has additions.

I think we need an int here with 0 | 1 as values

I don't like the idea of using an int as a boolean, but I've adjusted the command output error flag accordingly.
It is interesting to note that on my Rocky Linux eve, if I install cjson-devel I don't have the cJSON*.cmake files and hence the only way I found to compile pgagroal is to install the cJSON 1.7.14 library from sources.

fluca1978 · 2024-02-07T08:37:52Z

Rebased, but I don't know how to fix the max build error.

jesperpedersen · 2024-02-07T09:24:21Z

Did you try brew install cjson in .github/workflows/ci.yml ?

fluca1978 · 2024-02-07T10:10:56Z

Did you try brew install cjson in .github/workflows/ci.yml ?

Yes, but we got a linking error. Despite the fact that linux build is using cjson 1.7.15 and OSX is using 1.7.17, there are too much unknown symbols in the error, that make me think there are not the development files (something like cjson-dev).

jesperpedersen · 2024-02-07T10:19:14Z

@2010YOUY01 Do you have ideas ?

jesperpedersen · 2024-02-07T14:08:56Z

@fluca1978 Rocky 8 fails to compile - can you add a cmake/FindcJSON.cmake file ?

It uses

cjson-devel-1.7.14-2.el8.x86_64
cjson-1.7.14-2.el8.x86_64

jesperpedersen · 2024-02-07T14:32:32Z

Same for Rocky 9 w/

cjson-1.7.14-5.el9.x86_64
cjson-devel-1.7.14-5.el9.x86_64

jesperpedersen · 2024-02-08T02:37:43Z

@HazemRawi If you have ideas about the MacOS failure for this pull request feel free to add your comments

fluca1978 · 2024-02-08T10:08:09Z

@fluca1978 Rocky 8 fails to compile - can you add a cmake/FindcJSON.cmake file ?

It uses
cjson-devel-1.7.14-2.el8.x86_64
cjson-1.7.14-2.el8.x86_64

Added cmake/FindcJSON.cmake that works for me on a clean Rocky Liunx 9 installation.
I've also added the cJSON dependcy on the README.md and the packages installations to allow compilation.

fluca1978 · 2024-02-08T12:34:36Z

I made it!
After all, it was more trivial than I thought: the src/CMakeList.txt has a branch specific for OSX (darwin) and in such branch there was not the linking of the libraries related to cJSON.

I would like also to thank my friend Chris Mair that tried to help me for the setup.

jesperpedersen · 2024-02-08T14:54:41Z

src/include/json.h

@@ -0,0 +1,162 @@
+/*
+ * Copyright (C) 2023 Red Hat


There is a new copyright

.github/workflows/ci.yml

doc/CLI.md

src/include/json.h

jesperpedersen · 2024-02-08T14:58:32Z

src/include/json.h

+ * @param json the JSON object to analyze
+ * #returns the status message or NULL in case the JSON object is not valid
+ */
+const char* json_get_command_object_status(cJSON* json);


src/include/json.h

jesperpedersen · 2024-02-08T14:58:49Z

src/include/json.h

+ *
+ * @param json the json object to print
+ */
+void json_print_and_free_json_object(cJSON* json);


src/include/json.h

jesperpedersen · 2024-02-08T15:00:52Z

src/libpgagroal/json.c

@@ -0,0 +1,268 @@
+/*
+ * Copyright (C) 2023 Red Hat


New copyright header

fluca1978 · 2024-02-08T15:37:49Z

Commit ca8cb3d fixes the copyright and spaces. The only thing left is the pgagroal_ prefix in front of the cJSON utility functions: is this mandatory? Because those functions manipulates only cJSON objects.
If the pgagroal_ prefix is mandatory, then I'll apply in the next days.

jesperpedersen · 2024-02-08T15:47:41Z

Lets do the prefix - our .h could end up somewhere... Take your time

…roal-cli` This commit introduces the capaiblity for `pgagroal-cli` to print out the command results in a JSON format. A new dependency on the library 'cJSON' has been introduced; see <https://github.com/DaveGamble/cJSON>. A new set of functions, named 'pgagroal_json_xxx' have been added in a separated file json.c (include file json.h) to handle JSON structures in a more consistent way. Each function that manipulates a JSON object has been named with the 'pgagroal_json_' prefix. Each management function that reads data out of the protocol and creates or handles json data has been named with the prefix 'pgagroal_management_json_'. A few functions with a prefix name 'pgagroal_management_json_print' are used to print out a JSON object as normal text. Every command output, in the JSON format, is structured with a 'command' is the main object contained in the output, that in turns has different attributes: - 'output' is an object that contains the command specific information (e.g., list of databases, messages, and so on); - `error` a boolean-like value that indicates if the command was in error or not (0 means success, 1 means error); - `status` a string that contains either 'OK' or an error message in the case the `error` flag is set; - `exit-status` an integer value with the exit status of the command, 0 in case of success, a different value in case of failure. The JSON object for a result includes also another object, named 'application', that can be used for introspection: such object describes which executable has launched the command (so far, only `pgagroal-cli`) and at which version. In the 'output' object, every thing that has a list (e.g., connections, limits, databases, servers) will be wrapped into an object with the attributes 'list' and 'count': the former contains the details of every "thing", while the latter contains the count (i.e., the size of the array 'list' ). Whenever possible, a 'state' string is placed to indicate the state of the single entry. The command `status` and `status details` have been unified on the management side. There is a single function that handles both the cases of reading the answer for the `status` or the `status details` commands. This has been done because `status details` includes the output of `status`. The function `pgagroal_management_json_read_status_details` is in charge of getting the answer of the management protocol (i.e., read the socket) and invoke the printing function in the case the output is of type text. The above `pgagroal_management_json_read_status_details` returns always a JSON object, that can be converted back to the text output via `pgagroal_management_json_print_status_details`. In this way, the output between the JSON and the text formats are always coherent for these commands. The `ping` (i.e., `is-alive`) command does not print nothing by default. In the JSON version it provides the numerical status of the server and a string that represents a human-readable status. The `conf get` command has been refactored to be symmetric with other commands: the logic to print out the result is now within the management function (pgagroal_management_read_config_get) as per other commands. The JSON provides the `config-key` and the `config-value` as strings. See agroal#390 The `conf set` command has been refactored similarly to `conf get` in order to have all the logic to print out the information into the management read method (see agroal#390). The exit status provided by the command is now the result of the matching within the JSON object of the expected configuration value and the requested configuration value. The `conf ls` command has been refactored to produce a JSON object when reading data out of the management socket. Such JSON object is then printed as normal text if required. A new utility function, named 'pgagroal_server_status_as_string' has been added to the utils.c stuff. The idea is to have a consistent way to translate the numerical status representation into an human readable string. The text output format of commands has slightly changed due to the refactoring of some internal methods. Documentation updated. CI workflow updated. Library list updated. Added a FindcJSON.cmake file to help in finding out the library. Update copyright of the json files to current year and community. Close agroal#385 Close agroal#390

fluca1978 · 2024-02-08T16:49:08Z

All JSOn functions renamed to pgagroal_json_xxx in 8b13185

jesperpedersen · 2024-02-08T17:56:36Z

Merged. Great work !

Thanks for your contribution !

jesperpedersen self-requested a review December 7, 2023 08:14

jesperpedersen assigned fluca1978 Dec 7, 2023

jesperpedersen added the feature New feature label Dec 7, 2023

fluca1978 force-pushed the cjson branch from 83dd8ef to 85ea9d6 Compare December 7, 2023 08:29

fluca1978 force-pushed the cjson branch from 85ea9d6 to 87e763d Compare December 7, 2023 12:01

fluca1978 force-pushed the cjson branch 2 times, most recently from 3d6ca1f to c9a0275 Compare December 7, 2023 12:32

fluca1978 mentioned this pull request Dec 9, 2023

Add application version within the management protocol #392

Closed

fluca1978 force-pushed the cjson branch from c9a0275 to c4cfde5 Compare February 7, 2024 08:34

fluca1978 force-pushed the cjson branch from ec43660 to 7c42f2f Compare February 8, 2024 09:57

fluca1978 force-pushed the cjson branch 4 times, most recently from 6ccd745 to 1e47180 Compare February 8, 2024 12:29

jesperpedersen requested changes Feb 8, 2024

View reviewed changes

fluca1978 force-pushed the cjson branch from 1e47180 to ca8cb3d Compare February 8, 2024 15:36

fluca1978 force-pushed the cjson branch from ca8cb3d to 8b13185 Compare February 8, 2024 16:44

jesperpedersen merged commit 8b13185 into agroal:master Feb 8, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#385][#390] JSON command output implementation for `pgagroal-cli` #391

[#385][#390] JSON command output implementation for `pgagroal-cli` #391

fluca1978 commented Dec 6, 2023

jesperpedersen commented Dec 7, 2023

fluca1978 commented Dec 7, 2023

jesperpedersen commented Dec 7, 2023

fluca1978 commented Dec 7, 2023

jesperpedersen commented Dec 7, 2023

jesperpedersen commented Dec 7, 2023

fluca1978 commented Dec 7, 2023

fluca1978 commented Feb 7, 2024

jesperpedersen commented Feb 7, 2024

fluca1978 commented Feb 7, 2024

jesperpedersen commented Feb 7, 2024

jesperpedersen commented Feb 7, 2024

jesperpedersen commented Feb 7, 2024

jesperpedersen commented Feb 8, 2024

fluca1978 commented Feb 8, 2024

fluca1978 commented Feb 8, 2024 •

edited

Loading

jesperpedersen Feb 8, 2024

fluca1978 Feb 8, 2024

jesperpedersen Feb 8, 2024

jesperpedersen Feb 8, 2024

jesperpedersen Feb 8, 2024

fluca1978 commented Feb 8, 2024

jesperpedersen commented Feb 8, 2024

fluca1978 commented Feb 8, 2024

jesperpedersen commented Feb 8, 2024

[#385][#390] JSON command output implementation for pgagroal-cli #391

[#385][#390] JSON command output implementation for pgagroal-cli #391

Conversation

fluca1978 commented Dec 6, 2023

jesperpedersen commented Dec 7, 2023

fluca1978 commented Dec 7, 2023

jesperpedersen commented Dec 7, 2023

fluca1978 commented Dec 7, 2023

jesperpedersen commented Dec 7, 2023

jesperpedersen commented Dec 7, 2023

fluca1978 commented Dec 7, 2023

fluca1978 commented Feb 7, 2024

jesperpedersen commented Feb 7, 2024

fluca1978 commented Feb 7, 2024

jesperpedersen commented Feb 7, 2024

jesperpedersen commented Feb 7, 2024

jesperpedersen commented Feb 7, 2024

jesperpedersen commented Feb 8, 2024

fluca1978 commented Feb 8, 2024

fluca1978 commented Feb 8, 2024 • edited Loading

jesperpedersen Feb 8, 2024

Choose a reason for hiding this comment

fluca1978 Feb 8, 2024

Choose a reason for hiding this comment

jesperpedersen Feb 8, 2024

Choose a reason for hiding this comment

jesperpedersen Feb 8, 2024

Choose a reason for hiding this comment

jesperpedersen Feb 8, 2024

Choose a reason for hiding this comment

fluca1978 commented Feb 8, 2024

jesperpedersen commented Feb 8, 2024

fluca1978 commented Feb 8, 2024

jesperpedersen commented Feb 8, 2024

[#385][#390] JSON command output implementation for `pgagroal-cli` #391

[#385][#390] JSON command output implementation for `pgagroal-cli` #391

fluca1978 commented Feb 8, 2024 •

edited

Loading