Implement fbs schema & json to fbs converter tool - #738
Conversation
Thomas-Mikhael
commented
Jul 21, 2026
- Define the .fbs schema file that mirrors the existing JSON configuration structure.
- Build a JSON-to-FlatBuffer converter tool for generating FlatBuffer config from existing JSON configurations
- Flag to control compiling the Flatbuffers (--config=flatbuffers), default is off
- PyTest for the tool
+ Define the .fbs schema file that mirrors the existing JSON configuration structure. + Build a JSON-to-FlatBuffer converter tool for generating FlatBuffer config from existing JSON configurations + Flag to control compiling the Flatbuffers (--config=flatbuffers) + PyTest for the tool
a2ca224 to
2510ac0
Compare
|
As discussed via Slack - I love that we go for the Flatbuffers support and the contribution you perform here! But it would be very welcome to see the roadmap and map for its qualification before continuing here. Stating again: This does not require a full and ready qualification, we would just like to judge if the qualification at the end can succeed. |
Hi Jan, I agree. |
castler
left a comment
There was a problem hiding this comment.
Thanks this is going into the right direction.
I still believe that we need a flatbuffers certifcation for the 1.0 - and not only in 2027. And we should have a parallel workstream with weekly (or bi-weekly) reporting about the tasks there.
Anyhow - for this PR, my major concern is that we know have 4 places where we need to know the different schemas:
- In the JSON Parser (soonish also flatbuffers parser +1)
- In the JSON Schema
- In the Flatbuffers Schema
- In the JSON to Flatbuffers converter
I think this is to much duplication and will make it not maintainable. We should strive to have a generic flatbuffers schema generatioin and also a generci json to flatbuffers converter - to reduce the duplication as much as possible.
|
|
||
| # Enables FlatBuffers-based mw::com configuration tooling (disabled by default, | ||
| # see score/mw/com/impl/configuration/flatbuffers_flags.bzl). | ||
| common:flatbuffers --//score/mw/com/impl/configuration:enable_flatbuffers=true |
There was a problem hiding this comment.
Can we please name this experimental_enable_flatbuffer_configuration
| //score/mw/com/impl/configuration/converter:json_to_flatbuffer | ||
| //score/mw/com/impl/configuration/converter:mw_com_config.bin |
There was a problem hiding this comment.
We should guard this via the experimental flag - and since the experimental flag is per default off, these targets should net yet be publicly visible.
There was a problem hiding this comment.
Is it okay if I remove them from here and add them to EXCLUDED_PUBLIC_TARGETS in quality/visibility_guard/parser.py?
There was a problem hiding this comment.
No, as Long as it is Experimental it should Not be public
| the pinned ``@flatbuffers//:flatc`` compiler, so ``.bin`` artifacts become part | ||
| of the build graph and can be consumed by other targets (e.g. as ``data``). | ||
| """ | ||
|
|
There was a problem hiding this comment.
you are missing a visibility declaration in this *.bzl file
There was a problem hiding this comment.
I'll add it as
visibility(["//score/mw/com/impl/configuration/..."])
| // | ||
| // FlatBuffers schema mirroring the mw::com runtime configuration. | ||
| // | ||
| // This schema is the binary-serialization counterpart of the JSON configuration |
There was a problem hiding this comment.
I would have expected that we generate this file out of the JSON schema - otherwise, we get a problem with maintainability - as we have to take care in various places the the configuration is maintained.
There was a problem hiding this comment.
That would not be safe, JSON Schema is loosy and defines only the structure.
1- It doesn't specify the size (Integer widths: uint8, 16, ...)
For an ASIL / safety-relevant config, silently inferring a wrong width (e.g. int64 where the C++ side expects uint16) is exactly the kind of defect that wouldn't be caught by flatc.
2- Enum sentinels e.g. INVALID = 0 is a deliberate "unset detection" (design mirroring) for QualityType
3- = null optional-scalar vs baked default
4- Table names for nested objects (e.g. ServiceTypeEvent vs InstanceEvent)
5- json_to_flatbuffer.py hardcodes the same key->field mapping
So yes, maintaining 2 Schemas is not great, but it's the safest way here (with existing one common testing for both - to come in next stories)
Also, JSON Schema is not changing that often, mostly stable, right?
What I suggest, is to implement a "drift-detection test" that parses the JSON schema and the .fbs and fails the build if their structure diverges.
This catches if someone edits the JSON schema and forgets the .fbs.
What do you think?
There was a problem hiding this comment.
Its not only the two schemas, we will also have another instance of this mapping in the json to flatbuffer tool.
This will be (hopefully) a long-term solution, if flatbuffers is safety certified, and thus it will be a maintaince nightmare. I agree that the config does not change often, but it will change - and then you do not want to edit so many places.
I see multiple options:
1st) Have integers always as uint64. We have the very same problem with JSON and there we check the boundary values within the C++ code. If we go this way, it is the same mechanichs in JSON ant Flatbuffers. Same is true for enum sentinels or "null optional scalars. We should just handle both the same and not have defaults in the one, where we have optional values in the other
2nd) Generate the JSON schema from flatbuffers. As per my understanding this is directly supported by flatc. <- But this will be though since this will only work with "experimental flag turned on" most certainly?
There was a problem hiding this comment.
I'll investigate the second option as I see it more robust.
There was a problem hiding this comment.
I think we can have the JSON schema committed to the repo, and when a speial flag is on then it will be regenerated and overwritten, but this would be needed only with Schema change/update.
Then we will not need to maintain both schemas manually, but just the Flatbuffers schema and regenerate the JSON one.
Would this be okay if feasible?
There was a problem hiding this comment.
yes that would be better IMHO
There was a problem hiding this comment.
Ok, here's some analysis limitations for using fbs schema as our main schema to maintain and generate the JSON schema from it using 'flatc --jsonschema':
- Hyphen '-' is forbidden in fbs, so I need to convert it to '_' for example, and restore it when generating the JSON schema (doable)
- flatc --jsonschema output can't reproduce the rich hand-authored JSON schema (it loses titles/descriptions/defaults/min-max/string-enums)
To not tradeoff here, I'd try adding custom attributes to fbs and try writing a script that appends these attributes to the generated JSON Schema (will be much of code but if once done, would save the maintenance nightmare you mentioned)
I'll ping you once I have more updates
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
| """Convert an mw::com configuration JSON file into a FlatBuffer binary. |
There was a problem hiding this comment.
My hope is, that if we auto generate the *.fbs, we also get rid of this tool (or make it way more easy - as we do not have yet another place where we translate between the two tools
There was a problem hiding this comment.
This converter tool is only a facilitating option for whom already has a JSON config and wants to use flatbuffer config binaries for its benefits.
It's converting to flatbuffer convertable json format (snake case)
User has the freedom to use any other method to get flatbuffer binaries directly.
For example:
- Programmatic builder (FlatBufferBuilder in C++, the flatbuffers package in Python, etc.) with any other text formats: YAML, TOML, XML, CSV, .ini.
- A database or network service — query rows/records and build directly.
- In-memory C++ objects
There was a problem hiding this comment.
I understand that, but Here we have then again the key mapping. Thats why auch a tool ja fine, but it should ne generic without hardcoding again the mappings
Thomas-Mikhael
left a comment
There was a problem hiding this comment.
I added my replies/questions for each comment
| // | ||
| // FlatBuffers schema mirroring the mw::com runtime configuration. | ||
| // | ||
| // This schema is the binary-serialization counterpart of the JSON configuration |
There was a problem hiding this comment.
That would not be safe, JSON Schema is loosy and defines only the structure.
1- It doesn't specify the size (Integer widths: uint8, 16, ...)
For an ASIL / safety-relevant config, silently inferring a wrong width (e.g. int64 where the C++ side expects uint16) is exactly the kind of defect that wouldn't be caught by flatc.
2- Enum sentinels e.g. INVALID = 0 is a deliberate "unset detection" (design mirroring) for QualityType
3- = null optional-scalar vs baked default
4- Table names for nested objects (e.g. ServiceTypeEvent vs InstanceEvent)
5- json_to_flatbuffer.py hardcodes the same key->field mapping
So yes, maintaining 2 Schemas is not great, but it's the safest way here (with existing one common testing for both - to come in next stories)
Also, JSON Schema is not changing that often, mostly stable, right?
What I suggest, is to implement a "drift-detection test" that parses the JSON schema and the .fbs and fails the build if their structure diverges.
This catches if someone edits the JSON schema and forgets the .fbs.
What do you think?
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
| """Convert an mw::com configuration JSON file into a FlatBuffer binary. |
There was a problem hiding this comment.
This converter tool is only a facilitating option for whom already has a JSON config and wants to use flatbuffer config binaries for its benefits.
It's converting to flatbuffer convertable json format (snake case)
User has the freedom to use any other method to get flatbuffer binaries directly.
For example:
- Programmatic builder (FlatBufferBuilder in C++, the flatbuffers package in Python, etc.) with any other text formats: YAML, TOML, XML, CSV, .ini.
- A database or network service — query rows/records and build directly.
- In-memory C++ objects
| //score/mw/com/impl/configuration/converter:json_to_flatbuffer | ||
| //score/mw/com/impl/configuration/converter:mw_com_config.bin |
There was a problem hiding this comment.
Is it okay if I remove them from here and add them to EXCLUDED_PUBLIC_TARGETS in quality/visibility_guard/parser.py?
|
|
||
| # Enables FlatBuffers-based mw::com configuration tooling (disabled by default, | ||
| # see score/mw/com/impl/configuration/flatbuffers_flags.bzl). | ||
| common:flatbuffers --//score/mw/com/impl/configuration:enable_flatbuffers=true |
| the pinned ``@flatbuffers//:flatc`` compiler, so ``.bin`` artifacts become part | ||
| of the build graph and can be consumed by other targets (e.g. as ``data``). | ||
| """ | ||
|
|
There was a problem hiding this comment.
I'll add it as
visibility(["//score/mw/com/impl/configuration/..."])