Skip to content

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Mar 27, 2025

Refactors the management of Jinja2 templates, some notes on this PR.

  • The new Jinja2Template class is initialized by providing a template parameter as either a str or a pathlib.Path object, in the first case the template will be rendered from a string and the latter part will use file based templates
  • The rendering is done in an async manner in order to support async Jinja2 filters
  • The .get_variables() method will return a list of any variables that are required by the template, a current limitation for now is that for file based templates this is only supported for the first file and not if the template includes other files as imports (this part can be added later, I just want to have it in place for the string based computed attributes and it's not used anywhere today)
  • I reimplemented the identify_faulty_jinja_code() function with some minor modifications
  • In this PR I've added custom exceptions under the infrahub_sdk.template folder, I'm not completely sure we want to do this? They all have the base infrahub_sdk.exceptions.Error as an ancestor
  • Some older exceptions in infrahub_sdk.exceptions and the identify_faulty_jinja_code() will need to be deprecated

Note that we talked about providing a prefix for the netutils filters, though I'm not sure this is required now that we've listed the source of each filter within the documentation. Any thoughts around that?

@codecov
Copy link

codecov bot commented Mar 27, 2025

Codecov Report

Attention: Patch coverage is 90.78947% with 21 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...frahub_sdk/pytest_plugin/items/jinja2_transform.py 52.94% 7 Missing and 1 partial ⚠️
infrahub_sdk/ctl/render.py 75.86% 4 Missing and 3 partials ⚠️
infrahub_sdk/template/__init__.py 95.31% 3 Missing and 3 partials ⚠️
@@             Coverage Diff             @@
##           develop     #327      +/-   ##
===========================================
+ Coverage    71.46%   72.45%   +0.99%     
===========================================
  Files           87       91       +4     
  Lines         7990     8165     +175     
  Branches      1544     1572      +28     
===========================================
+ Hits          5710     5916     +206     
+ Misses        1875     1838      -37     
- Partials       405      411       +6     
Flag Coverage Δ
integration-tests 22.27% <0.00%> (-0.49%) ⬇️
python-3.10 46.56% <63.15%> (+0.89%) ⬆️
python-3.11 46.54% <63.15%> (+0.84%) ⬆️
python-3.12 46.54% <63.15%> (+0.87%) ⬆️
python-3.13 46.54% <63.15%> (+0.84%) ⬆️
python-3.9 44.90% <50.87%> (+0.30%) ⬆️
python-filler-3.12 25.08% <41.22%> (+0.47%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/ctl/cli_commands.py 68.59% <100.00%> (+10.45%) ⬆️
infrahub_sdk/template/exceptions.py 100.00% <100.00%> (ø)
infrahub_sdk/template/filters.py 100.00% <100.00%> (ø)
infrahub_sdk/template/models.py 100.00% <100.00%> (ø)
infrahub_sdk/template/__init__.py 95.31% <95.31%> (ø)
infrahub_sdk/ctl/render.py 69.44% <75.86%> (+26.58%) ⬆️
...frahub_sdk/pytest_plugin/items/jinja2_transform.py 60.29% <52.94%> (+5.49%) ⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added the type/documentation Improvements or additions to documentation label Mar 31, 2025
@ogenstad ogenstad force-pushed the pog-jinja2-refactor branch 4 times, most recently from 78400a5 to cb7816b Compare March 31, 2025 12:02
Copy link
Contributor

@BaptisteGi BaptisteGi left a comment

Choose a reason for hiding this comment

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

I think it would be great to point to that reference page from the main infrahub documentation, I'm thinking about (might be some other places):

  • Jinja2 computed attributes
  • Jinja2 transform (artifact)

---


## Builtin Jinja2 filters
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth pointing to the doc on this one (https://jinja.palletsprojects.com/en/stable/templates/#list-of-builtin-filters) so users know what's behind each line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added these links.

@ogenstad ogenstad force-pushed the pog-jinja2-refactor branch from cb7816b to 66ac470 Compare April 1, 2025 09:13
@ogenstad ogenstad merged commit e833024 into develop Apr 1, 2025
17 checks passed
@ogenstad ogenstad deleted the pog-jinja2-refactor branch April 1, 2025 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants