Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

requires/types: introduce expression type (expr.py) #913

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xmkg
Copy link
Contributor

@xmkg xmkg commented Jul 2, 2024

expression is a new requirement type that allows users to express a requirement in human-readable, SQL-like syntax. It has proper grammar to allow users to write checks in as easily digestible manner, which will improve the QOL for scenario writers and code reviewers. The grammar is constructed using the pyparsing library.

Example:

checks:
  not_using_iommu_passthrough_for_suitable_amd: |
    (@hotsos.core.plugins.kernel.sysfs.CPU.vendor == 'authenticamd') AND
    (@hotsos.core.plugins.system.SystemBase.virtualisation_type == None) AND
    NOT('iommu=pt' IN @hotsos.core.plugins.kernel.KernelBase.boot_parameters)    

With the alias patch, it'll look much prettier:

checks:
  not_using_iommu_passthrough_for_suitable_amd: |
    (@kernel.sysfs.cpu.vendor == 'authenticamd') AND
    (@system.virtualisation_type == None) AND
    NOT('iommu=pt' IN @kernel.boot_parameters)    

The current grammar supports the following constructs:

  • Keywords (True, False, None)
  • Constants (Integer, Float, String literal)
  • Runtime variables (Python properties)
  • Functions(len, not, file, systemd, read_ini, read_cert)
  • Arithmetic operators(sign, mul, div, add, sub, exp)
  • Comparison operators(<, <=, >, >=, ==, !=, in)
  • Logical operators(and, or, not)
  • Comments('#', '//', '/.../')

Also, the following changes have been made:

  • Updated requirement_types.rst to document the new Expression requirement type.
  • Moved property resolver logic from YPropertyBase to PythonEntityResolver class.
  • Added pyparsing as a dependency.
  • Added unit tests for the new code.

This patch rewrites some of the scenario checks in the new expression syntax to demonstrate the difference and provide examples.

@xmkg
Copy link
Contributor Author

xmkg commented Jul 2, 2024

IDK why the "Test / test (3.8, ubuntu-20.04) (pull_request)" job is failing, all tests are green in my env. I'll re-run them in a py3.8 environment and get to the bottom of it.

@xmkg xmkg force-pushed the feature/add-expression-keyword-to-checks branch 3 times, most recently from 79b363d to a993c18 Compare July 4, 2024 16:17
@xmkg
Copy link
Contributor Author

xmkg commented Jul 4, 2024

@dosaboy fixed CI issues, ready for review. please feel free to ping me should you need to ask anything.

@xmkg
Copy link
Contributor Author

xmkg commented Jul 7, 2024

This feature is intended to be used in conjunction with the aliasing feature: #929

@xmkg xmkg force-pushed the feature/add-expression-keyword-to-checks branch from a993c18 to 6516845 Compare July 12, 2024 07:22
@xmkg xmkg force-pushed the feature/add-expression-keyword-to-checks branch from 6516845 to 2eb6c70 Compare July 30, 2024 11:25
@xmkg xmkg force-pushed the feature/add-expression-keyword-to-checks branch from 2eb6c70 to 86d2594 Compare July 30, 2024 12:06
Expression
----------

Expressions allows the user to express a requirement in a human-readable domain specific language. It's purpose
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Expressions allows the user to express a requirement in a human-readable domain specific language. It's purpose
Expressions allow the user to express a requirement in a human-readable domain specific language. Its purpose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

----------

Expressions allows the user to express a requirement in a human-readable domain specific language. It's purpose
is to provide a simplified, clear an concise expression grammar to all requirement types, and eliminate redundancies.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
is to provide a simplified, clear an concise expression grammar to all requirement types, and eliminate redundancies.
is to provide a simplified, clear and concise expression grammar for all requirement types and to eliminate redundancies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -106,11 +106,13 @@ def huge_pages_enabled(self):

@property
def hugetlb_to_mem_total_percentage(self):
return round((self.Hugetlb * 100) / self.MemTotal)
return (self.MemTotal and round(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add or 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's redundant since (int and int) would yield an int in either case.

PS for future travelers: The code is written this way to avoid division by zero.

@xmkg xmkg force-pushed the feature/add-expression-keyword-to-checks branch from 86d2594 to 3d41ed8 Compare July 30, 2024 15:51
@xmkg xmkg force-pushed the feature/add-expression-keyword-to-checks branch 2 times, most recently from c5efc64 to 06a1664 Compare August 16, 2024 07:56
`expression` is a new requirement type that allows user to express a requirement
in human-readable, SQL-like syntax. It has a proper grammar to allow user to write
checks as easily digestable manner, which will improve the QOL for scenario writers
and code reviewers. The grammar is constructed using `pyparsing` library.

The current grammar supports the following constructs:

- Keywords (True, False, None)
- Constants (Integer, Float, String literal)
- Runtime variables (Python properties)
- Functions(len, not, file, systemd, read_ini, read_cert)
- Arithmetic operators(sign, mul, div, add, sub, exp)
- Comparison operators(<, <=, >, >=, ==, !=, in)
- Logical operators(and, or, not)
- Comments('#', '//', '/*...*/')

Also, the following changes have been made:

- Updated requirement_types.rst to document the new Expression requirement type.
- Moved property resolver logic from YPropertyBase to PythonEntityResolver class.
- Added `pyparsing` as a dependency.
- Added unit tests for the new code, the coverage rate is 100 pct for the expr.py

This patch rewrites some of the scenario checks in the new expression syntax to
demonstrate the difference and provide examples.

Signed-off-by: Mustafa Kemal Gilor <[email protected]>
@xmkg xmkg force-pushed the feature/add-expression-keyword-to-checks branch from 06a1664 to a4b7750 Compare August 16, 2024 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants