Repository navigation
Conversation
nickrolfe
force-pushed
the
nickrolfe/mad-generator-json
branch
2 times, most recently
from
October 12, 2026 10:01
95c94ef to
ccf9c6e
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Bulk generation currently raises an attribute error, and single-file generation no longer produces a single file.
1 open finding
What changed in this PR
Refactors MaD generation around a format-independent representation and adds JSON output support.
Changes:
- Adds shared YAML/JSON serialization.
- Refactors generators and converters to accumulate typed rows.
- Cleans up generated files in either format.
| File | Description |
|---|---|
helpers.py |
Adds shared extension merging and writing. |
generate_mad.py |
Adds format selection and uses structured rows. |
data_extension_file.py |
Implements YAML and JSON serialization. |
convert_extensions.py |
Uses the shared extension representation. |
bulk_generate_mad.py |
Cleans up YAML and JSON outputs. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
In preparation for adding support for JSON data extensions, this commit updates the scripts so that they store and build models using an in-memory representation that is independent of the YAML format currently used. This means rows are represented as Python tuples, where previously they were created as YAML-formatted strings in the merging process. Scalar values inside those tuples are represented as Python strings and booleans. Only the DataExtensionFile.write_yaml function now has to deal with YAML formatting concerns.
nickrolfe
force-pushed
the
nickrolfe/mad-generator-json
branch
from
October 12, 2026 10:28
ccf9c6e to
893bc80
Compare
The `generate_mad.py` script accepts a flag that lets you choose between YAML or JSON, but YAML is still the default. We can change the default later, when CLI support for JSON is widely available. `convert_extensions.py` also still hard-codes `.yml` as the output extension/format.
nickrolfe
force-pushed
the
nickrolfe/mad-generator-json
branch
from
October 12, 2026 10:52
893bc80 to
3872102
Compare
nickrolfe
marked this pull request as ready for review
October 12, 2026 10:59
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is split into two commits:
DataExtensionFile.write_yamlfunction now has to deal with YAML formatting concerns.For now, YAML is still the default. We can change this when CLI support for JSON extensions is widely available and the existing models in this repo have been converted to JSON. (I will be making a separate PR with a conversion script).
I tested these changes by running both
misc/scripts/models-as-data/generate_mad.pyandjava/ql/src/utils/modelconverter/ConvertExtensions.pyand manually inspecting the output.