Update protocols to support Template and fix support for Sync #348
Conversation
github-actions
bot
added
the
type/documentation
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #348 +/- ##
===========================================
+ Coverage 72.45% 73.04% +0.59%
===========================================
Files 91 92 +1
Lines 8180 8200 +20
Branches 1576 1583 +7
===========================================
+ Hits 5927 5990 +63
+ Misses 1841 1797 -44
- Partials 412 413 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
dgarros
force-pushed
the
dga-20250407-protocols
branch
from
fbbef32 to
38b3719
Compare
Deploying infrahub-sdk-python with
|
| Latest commit: |
d107906
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://65a2327a.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://dga-20250407-protocols.infrahub-sdk-python.pages.dev |
dgarros
force-pushed
the
dga-20250407-protocols
branch
from
38b3719 to
785fe6e
Compare
| jinja2_env.filters["syncify"] = self._jinja2_filter_syncify | ||
|
|
||
| template = jinja2_env.from_string(PROTOCOLS_TEMPLATE) | ||
| template = jinja2_env.from_string(load_template()) |
There was a problem hiding this comment.
I don't think we need to change this in this PR but I think it would make sense to the templating logic we have elsewhere within the SDK to render these templates. Possibly we should come to a final decision regarding #337 first if we should leave it as async by default or not.
| @task | ||
| def lint_vale(context: Context) -> None: | ||
| """Run vale to check all documentation files.""" | ||
| if not is_tool_installed("vale"): |
There was a problem hiding this comment.
I'm not sure about these is_tool_installed, it feels like we at some point could get into a situation where we're just running the lint_all command and we move around something so that these tools aren't installed. Then we wouldn't notice it in the pipeline.
There was a problem hiding this comment.
If someone doesn't have the right tools installed, the pipeline will still catch the issue right now.
These methods are mainly for us to make it easier to validate a branch locally but since not everyone will need them I would prefer to not require everyone to install vale & markdownlint-cli2
| ) | ||
| async def test_filter_syncify(sync, input, output): | ||
| assert CodeGenerator._jinja2_filter_syncify(value=input, sync=sync) == output | ||
| assert CodeGenerator._jinja2_filter_syncify(value=input, sync=sync) == output |
There was a problem hiding this comment.
I liked this pattern of setting up dataclasses for the test cases, I think it adds clarity with regards to what the expected input fields are along with the possibility to give each test a relevant name.
https://github.com/opsmill/infrahub-sdk-python/blob/v1.10.2/tests/unit/sdk/test_template.py#L45-L81
There was a problem hiding this comment.
Good point I've updated the test
dgarros
force-pushed
the
dga-20250407-protocols
branch
from
785fe6e to
d107906
Compare
Fixes #329
This PR includes a few enhancements and fixes around Python Protocols
This PR also adds some invoke commands to help with the docs
invoke lint-docinvoke lint-valeinvoke lint-markdownlintIf
valeand/ormarkdownlint-cli2are not installed, these commands will be skipped