-
Notifications
You must be signed in to change notification settings - Fork 739
Pre-load Jinja macros #13502
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
Merged
Merged
Pre-load Jinja macros #13502
Conversation
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
The "products" variable is set by build system to the value of "product" variable, which is constant during the build of content for a product.
Macros defined in `harden_ssh_client_crypto_policy/oval/shared.xml` will be renamed to prevent conflicts with macros of the same name defined in `shared/macros/10-oval.jinja`.
The macro `oval_metadata` might be already loaded from the time when there was a different `product` value in scope. To ensure that this old value doesn't affect the generated OVAL we will specify affected platforms explicitely.
/packit retest-failed |
1 similar comment
/packit retest-failed |
Reduce cognitive complexity by extracting a function and inverting a condition.
Code Climate has analyzed commit 9b1dae9 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 88.8% (50% is the threshold). This pull request will bring the total coverage in the repository to 61.8% (-0.1% change). View more on Code Climate. |
Mab879
approved these changes
May 29, 2025
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.
Description:
Currently, the scripts building our content spend most of the time by loading Jinja macro definitions. Before processing each file, all Jinja macro definitions need to be reloaded, which means they are loaded more than thousand times.
The build could be optimized by loading the macros only once at the beginning of processing and then reusing them during the build.
Unfortunately, this can't be done with our current Jinja macros. Some of our macros use global variables like
rule_id
,rule_title
. The problem is once a macro is loaded by Jinja, the global variables are permanently bound to values at the time of the loading and the binding can't be changed later. Only variables that are macro arguments can be changed dynamically. This means that to be able to reuse already loaded macros, all variables that are not constant need to be converted to macros parameters and their values always need to be passed from the Jinja template.Therefore, as a part of this PR, we will replace all occurences of non-constant global variables by macro parameters and we will pass these parameters to macros in all our files where the changed macros are used. This is a massive change but not complex - the affected macros simply got new parameters
rule_id
orrule_title
or both.The changes in macro definitions and macro invocations have been done automatically using a script. This script is
change_jinja_signatures.py
and is added and then removed by this PR.After the change of macros, this PR changes the build system logic to pre-load the Jinja macros at the beginning of the Python build scripts that perform the Jinja rendering so that the macros are loaded just once in each script.
Rationale:
This PR brings significant speed improvements of the build process. Specifically,
./build_product -d rhel9
takes 18 s instead of 33 s on a laptop. The speed up enhances developer experience and saves resources.Review Hints:
change_jinja_signatures.py
. Run unit tests in this script usingpython3 -m pytest change_jinja_signatures.py
.