Skip to content

ENT-13814: Added syntax-description and GH Action to update it weekly to replace policy_language.py#59

Merged
olehermanse merged 1 commit intocfengine:mainfrom
SimonThalvorsen:main
Apr 13, 2026
Merged

ENT-13814: Added syntax-description and GH Action to update it weekly to replace policy_language.py#59
olehermanse merged 1 commit intocfengine:mainfrom
SimonThalvorsen:main

Conversation

@SimonThalvorsen
Copy link
Copy Markdown
Contributor

Adds syntax-description.json
Removes old policy_language.py
Uses syntax-decription.json to get BUILTINS

… to replace policy_language.py

Adds syntax-description.json
Removes old policy_language.py
Uses syntax-decription.json to get BUILTINS

Signed-off-by: Simon Halvorsen <simon.halvorsen@northern.tech>
Comment on lines +53 to +77
def _derive_syntax_sets(data: dict) -> tuple:
"""Derive the four sets used for linting from a loaded syntax-description dict.

Returns: (ALLOWED_BUNDLE_TYPES, BUILTIN_PROMISE_TYPES, BUILTIN_FUNCTIONS, DEPRECATED_PROMISE_TYPES)
"""
builtin_body_types = set(data.get("bodyTypes", {}).keys())

allowed_bundle_types = data.get("bundleTypes", {}).keys()

builtin_promise_types = set(data.get("promiseTypes", {}).keys())

builtin_functions = set(data.get("functions", {}).keys())

deprecated_promise_types = {
"defaults",
"guest_environments",
} # Has to be hardcoded, not tagged in syntax-description.json

return (
builtin_body_types,
allowed_bundle_types,
builtin_promise_types,
builtin_functions,
deprecated_promise_types,
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, we will want to access the syntax data in more complicated ways, not just lists of keys. I don't think this approach will scale nicely, please use a class instead;

class SyntaxData:
  def __init__(self, path):
    self._data = get_json(path)
    # Maybe add some assertions here, linting will not work correctly
    # if for example allowed bundle types is empty.
    [...]
  def get_bundle_types(self):
    [...]
  # In the future we might choose to do something like;
  def is_valid_attribute(self, promise_type, attribute_name, attribute_value):
    [...]

@@ -0,0 +1,69 @@
name: Update syntax-description
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably prefer to put the JSON / yaml in as separate commits to avoid doing too many things in one commit.

"""
builtin_body_types = set(data.get("bodyTypes", {}).keys())

allowed_bundle_types = data.get("bundleTypes", {}).keys()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned below, linting will not work correctly if this one is empty, so it might be appropriate to assert / fail early when the syntax data is invalid / missing.

Copy link
Copy Markdown
Member

@olehermanse olehermanse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct, adjustments can be made in follow-up PRs.

@olehermanse olehermanse merged commit fae3057 into cfengine:main Apr 13, 2026
6 checks passed
Comment on lines +51 to +55
if ! cmp -s new.json ./src/cfengine_cli/syntax-description.json; then
cat new.json > ./src/cfengine_cli/syntax-description.json
echo "CHANGES_DETECTED=true" >> $GITHUB_ENV
rm new.json
fi
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ! cmp -s new.json ./src/cfengine_cli/syntax-description.json; then
cat new.json > ./src/cfengine_cli/syntax-description.json
echo "CHANGES_DETECTED=true" >> $GITHUB_ENV
rm new.json
fi
if ! cmp -s new.json ./src/cfengine_cli/syntax-description.json; then
cat new.json > ./src/cfengine_cli/syntax-description.json
echo "CHANGES_DETECTED=true" >> $GITHUB_ENV
rm new.json
fi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants