[RFC v1 0/1] dts: separate allowed values from json schema

[RFC v1 0/1] dts: separate allowed values from json schema

[Nicholas Pratte]

I started discussions with Juraj about potentially taking all of the
'test_suite' enums in the framework's json schema and putting them into
its own smaller json file that can easily be referenced. The thought
being that doing so might make it easier for outside developers to
navigate when writing their own test suites in the future, and it might
help to keep the schema clean in the long term as the list inevitably
gets larger. We took the idea one step further and discussed the idea of
putting all enum/allowed values within the schema into a different
location.

This is a pretty simple change that doesn't require much effort, but
separating the schema is not so graceful insofar that you cannot use
'$ref' tags like you might expect; the only way to do this reasonably
using Warlock is to merge the dictionaries together before creating a
Warlock model, hence the use of '$defs' instead of references to a local
file.

Nicholas Pratte (1):
  dts: split enums from primary json schema

 dts/framework/config/__init__.py              |   4 +
 dts/framework/config/conf_allowed_values.json | 131 ++++++++++++++++++
 dts/framework/config/conf_yaml_schema.json    | 111 ++-------------
 3 files changed, 143 insertions(+), 103 deletions(-)
 create mode 100644 dts/framework/config/conf_allowed_values.json

-- 
2.44.0

[RFC v1 1/1] dts: split enums from primary json schema

[Nicholas Pratte]

Separating enums from the primary json schema might make it easier for
incoming DTS developers and users to quickly identify what values are
allowed in the config yaml file with relative ease. Moreover, separating
values, such as available test suites, may streamline the test suite
development process via a quick and simple reference to test suite
enums.

The other concern is that, as more test suites are developed, then the
list of available test suites will become quite cumbersome. A list of
possible test suites should be well-documented and easy to find,
and separating test suite enums from the primary schema can achieve both
of these needs.

Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 dts/framework/config/__init__.py              |   4 +
 dts/framework/config/conf_allowed_values.json | 131 ++++++++++++++++++
 dts/framework/config/conf_yaml_schema.json    | 111 ++-------------
 3 files changed, 143 insertions(+), 103 deletions(-)
 create mode 100644 dts/framework/config/conf_allowed_values.json

diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index df60a5030e..8af81f2c29 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -593,9 +593,13 @@ def load_config(config_file_path: Path) -> Configuration:
         config_data = yaml.safe_load(f)
 
     schema_path = os.path.join(Path(__file__).parent.resolve(), "conf_yaml_schema.json")
+    allowed_values_path = os.path.join(Path(__file__).parent.resolve(), "conf_allowed_values.json")
 
     with open(schema_path, "r") as f:
         schema = json.load(f)
+    with open(allowed_values_path, "r") as f:
+        allowed_values = json.load(f)
+    schema.update(allowed_values)
     config = warlock.model_factory(schema, name="_Config")(config_data)
     config_obj: Configuration = Configuration.from_dict(dict(config))  # type: ignore[arg-type]
     return config_obj
diff --git a/dts/framework/config/conf_allowed_values.json b/dts/framework/config/conf_allowed_values.json
new file mode 100644
index 0000000000..243d6daf8d
--- /dev/null
+++ b/dts/framework/config/conf_allowed_values.json
@@ -0,0 +1,131 @@
+{
+  "$defs": {
+  "description": "Allowed/supported values and test suites within the current DTS framework.",
+    "test_suites": {
+      "description": "Developed test suites within the current framework. Add additional test suites here.",
+      "enum": [
+        "hello_world",
+        "os_udp",
+        "pmd_buffer_scatter"
+      ]
+    },
+    "traffic_generators": {
+      "description": "Scapy traffic generator. Used for functional testing.",
+      "enum": [
+        "SCAPY"
+      ]
+    },
+    "node_architectures": {
+      "description": "Supported node architectures within DPDK.",
+      "enum": [
+        "x86_64",
+        "arm64",
+        "ppc64le"
+      ]
+    },
+    "build_target_architectures": {
+      "description": "Supported build-target architectures.",
+      "enum": [
+        "ALL",
+        "x86_64",
+        "arm64",
+        "ppc64le",
+        "other"
+      ]
+    },
+    "operating_systems": {
+      "description": "Supported node operating systems",
+      "enum": [
+        "linux"
+      ]
+    },
+    "cpus": {
+      "description": "Supported build-target CPUs",
+      "enum": [
+        "native",
+        "armv8a",
+        "dpaa2",
+        "thunderx",
+        "xgene1"
+      ]
+    },
+    "compilers": {
+      "description": "Supported build-target compilers.",
+      "enum": [
+        "gcc",
+        "clang",
+        "icc",
+        "mscv"
+      ]
+    },
+    "NICs": {
+      "description": "Supported NICs",
+      "enum": [
+        "ALL",
+        "ConnectX3_MT4103",
+        "ConnectX4_LX_MT4117",
+        "ConnectX4_MT4115",
+        "ConnectX5_MT4119",
+        "ConnectX5_MT4121",
+        "I40E_10G-10G_BASE_T_BC",
+        "I40E_10G-10G_BASE_T_X722",
+        "I40E_10G-SFP_X722",
+        "I40E_10G-SFP_XL710",
+        "I40E_10G-X722_A0",
+        "I40E_1G-1G_BASE_T_X722",
+        "I40E_25G-25G_SFP28",
+        "I40E_40G-QSFP_A",
+        "I40E_40G-QSFP_B",
+        "IAVF-ADAPTIVE_VF",
+        "IAVF-VF",
+        "IAVF_10G-X722_VF",
+        "ICE_100G-E810C_QSFP",
+        "ICE_25G-E810C_SFP",
+        "ICE_25G-E810_XXV_SFP",
+        "IGB-I350_VF",
+        "IGB_1G-82540EM",
+        "IGB_1G-82545EM_COPPER",
+        "IGB_1G-82571EB_COPPER",
+        "IGB_1G-82574L",
+        "IGB_1G-82576",
+        "IGB_1G-82576_QUAD_COPPER",
+        "IGB_1G-82576_QUAD_COPPER_ET2",
+        "IGB_1G-82580_COPPER",
+        "IGB_1G-I210_COPPER",
+        "IGB_1G-I350_COPPER",
+        "IGB_1G-I354_SGMII",
+        "IGB_1G-PCH_LPTLP_I218_LM",
+        "IGB_1G-PCH_LPTLP_I218_V",
+        "IGB_1G-PCH_LPT_I217_LM",
+        "IGB_1G-PCH_LPT_I217_V",
+        "IGB_2.5G-I354_BACKPLANE_2_5GBPS",
+        "IGC-I225_LM",
+        "IGC-I226_LM",
+        "IXGBE_10G-82599_SFP",
+        "IXGBE_10G-82599_SFP_SF_QP",
+        "IXGBE_10G-82599_T3_LOM",
+        "IXGBE_10G-82599_VF",
+        "IXGBE_10G-X540T",
+        "IXGBE_10G-X540_VF",
+        "IXGBE_10G-X550EM_A_SFP",
+        "IXGBE_10G-X550EM_X_10G_T",
+        "IXGBE_10G-X550EM_X_SFP",
+        "IXGBE_10G-X550EM_X_VF",
+        "IXGBE_10G-X550T",
+        "IXGBE_10G-X550_VF",
+        "brcm_57414",
+        "brcm_P2100G",
+        "cavium_0011",
+        "cavium_a034",
+        "cavium_a063",
+        "cavium_a064",
+        "fastlinq_ql41000",
+        "fastlinq_ql41000_vf",
+        "fastlinq_ql45000",
+        "fastlinq_ql45000_vf",
+        "hi1822",
+        "virtio"
+      ]
+    }
+  }
+}
\ No newline at end of file
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..f148943cef 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -8,107 +8,25 @@
     },
     "NIC": {
       "type": "string",
-      "enum": [
-        "ALL",
-        "ConnectX3_MT4103",
-        "ConnectX4_LX_MT4117",
-        "ConnectX4_MT4115",
-        "ConnectX5_MT4119",
-        "ConnectX5_MT4121",
-        "I40E_10G-10G_BASE_T_BC",
-        "I40E_10G-10G_BASE_T_X722",
-        "I40E_10G-SFP_X722",
-        "I40E_10G-SFP_XL710",
-        "I40E_10G-X722_A0",
-        "I40E_1G-1G_BASE_T_X722",
-        "I40E_25G-25G_SFP28",
-        "I40E_40G-QSFP_A",
-        "I40E_40G-QSFP_B",
-        "IAVF-ADAPTIVE_VF",
-        "IAVF-VF",
-        "IAVF_10G-X722_VF",
-        "ICE_100G-E810C_QSFP",
-        "ICE_25G-E810C_SFP",
-        "ICE_25G-E810_XXV_SFP",
-        "IGB-I350_VF",
-        "IGB_1G-82540EM",
-        "IGB_1G-82545EM_COPPER",
-        "IGB_1G-82571EB_COPPER",
-        "IGB_1G-82574L",
-        "IGB_1G-82576",
-        "IGB_1G-82576_QUAD_COPPER",
-        "IGB_1G-82576_QUAD_COPPER_ET2",
-        "IGB_1G-82580_COPPER",
-        "IGB_1G-I210_COPPER",
-        "IGB_1G-I350_COPPER",
-        "IGB_1G-I354_SGMII",
-        "IGB_1G-PCH_LPTLP_I218_LM",
-        "IGB_1G-PCH_LPTLP_I218_V",
-        "IGB_1G-PCH_LPT_I217_LM",
-        "IGB_1G-PCH_LPT_I217_V",
-        "IGB_2.5G-I354_BACKPLANE_2_5GBPS",
-        "IGC-I225_LM",
-        "IGC-I226_LM",
-        "IXGBE_10G-82599_SFP",
-        "IXGBE_10G-82599_SFP_SF_QP",
-        "IXGBE_10G-82599_T3_LOM",
-        "IXGBE_10G-82599_VF",
-        "IXGBE_10G-X540T",
-        "IXGBE_10G-X540_VF",
-        "IXGBE_10G-X550EM_A_SFP",
-        "IXGBE_10G-X550EM_X_10G_T",
-        "IXGBE_10G-X550EM_X_SFP",
-        "IXGBE_10G-X550EM_X_VF",
-        "IXGBE_10G-X550T",
-        "IXGBE_10G-X550_VF",
-        "brcm_57414",
-        "brcm_P2100G",
-        "cavium_0011",
-        "cavium_a034",
-        "cavium_a063",
-        "cavium_a064",
-        "fastlinq_ql41000",
-        "fastlinq_ql41000_vf",
-        "fastlinq_ql45000",
-        "fastlinq_ql45000_vf",
-        "hi1822",
-        "virtio"
-      ]
+      "$ref": "#/$defs/NICs"
     },
 
     "ARCH": {
       "type": "string",
-      "enum": [
-        "x86_64",
-        "arm64",
-        "ppc64le"
-      ]
+      "$ref": "#/$defs/node_architectures"
     },
     "OS": {
       "type": "string",
-      "enum": [
-        "linux"
-      ]
+      "$ref": "#/$defs/operating_systems"
     },
     "cpu": {
       "type": "string",
       "description": "Native should be the default on x86",
-      "enum": [
-        "native",
-        "armv8a",
-        "dpaa2",
-        "thunderx",
-        "xgene1"
-      ]
+      "$refs": "#/$defs/cpus"
     },
     "compiler": {
       "type": "string",
-      "enum": [
-        "gcc",
-        "clang",
-        "icc",
-        "mscv"
-      ]
+      "$ref": "#/$defs/compilers"
     },
     "build_target": {
       "type": "object",
@@ -116,13 +34,7 @@
       "properties": {
         "arch": {
           "type": "string",
-          "enum": [
-            "ALL",
-            "x86_64",
-            "arm64",
-            "ppc64le",
-            "other"
-          ]
+          "$ref": "#/$defs/build_target_architectures"
         },
         "os": {
           "$ref": "#/definitions/OS"
@@ -184,11 +96,7 @@
     },
     "test_suite": {
       "type": "string",
-      "enum": [
-        "hello_world",
-        "os_udp",
-        "pmd_buffer_scatter"
-      ]
+      "$ref": "#/$defs/test_suites"
     },
     "test_target": {
       "type": "object",
@@ -298,13 +206,10 @@
             "oneOf": [
               {
                 "type": "object",
-                "description": "Scapy traffic generator. Used for functional testing.",
                 "properties": {
                   "type": {
                     "type": "string",
-                    "enum": [
-                      "SCAPY"
-                    ]
+                    "$ref": "#/$defs/traffic_generators"
                   }
                 }
               }
-- 
2.44.0

Re: [RFC v1 0/1] dts: separate allowed values from json schema

[Luca Vizzarro]

Hi Nicholas,

Thank you for the proposal. The way I see it is that JSON schema can be 
very developer unfriendly to read, therefore I would not rely on it for 
this purpose. Adding $refs as well could start making it more 
complicated for this purpose. It's helpful for structuring and 
maintaining the files though.

Normally, a schema is meant for validation purposes. And docs should 
provide the developer with the right information.

Nonetheless, I am getting started in replacing Warlock with Pydantic. 
And basing the Python classes on Pydantic models. Unfortunately, this 
will nullify this work altogether, as Pydantic provides an automatic 
facility for generating the JSON schema, thus removing the need for 
manual maintenance.

Best,
Luca

Re: [RFC v1 1/1] dts: split enums from primary json schema

[Jeremy Spewock]

Hey Nick,

Thanks for the patch. The way you did things all makes sense to me and
I like the idea of pulling out the definitions to make things more
compact in the main schema, the one thought I did have though was that
it isn't immediately obvious in the main schema where these values are
coming from. Because they just reference a $defs key that doesn't
exist in this file, there is no reference to something like "hey, go
look here for the definitions." Unfortunately, because it's JSON, you
also can't just leave a comment, so I guess the best we can do is just
make sure to document it well. I wonder if there is a way though that
you can have the paths for the references reference other files. I'm
sure you would have looked into that already, but that probably would
fix the confusion problem, and it stops you from having to recombine
manually.

<snip>

Re: [RFC v1 0/1] dts: separate allowed values from json schema

[Juraj Linkeš]

On 7. 8. 2024 17:01, Luca Vizzarro wrote:
> Hi Nicholas,
> 
> Thank you for the proposal. The way I see it is that JSON schema can be 
> very developer unfriendly to read, therefore I would not rely on it for 
> this purpose. Adding $refs as well could start making it more 
> complicated for this purpose. It's helpful for structuring and 
> maintaining the files though.
> 
> Normally, a schema is meant for validation purposes. And docs should 
> provide the developer with the right information.
> 
> Nonetheless, I am getting started in replacing Warlock with Pydantic. 
> And basing the Python classes on Pydantic models. Unfortunately, this 
> will nullify this work altogether, as Pydantic provides an automatic 
> facility for generating the JSON schema, thus removing the need for 
> manual maintenance.
> 
> Best,
> Luca

What Luca says makes a lot of sense. Let's not bother improving the 
schema since it'll be obsolete soon with the Pydantic changes.