refactor: Jinja experiment for AffiliateLinks template#12776
Open
Sanket17052006 wants to merge 5 commits into
Open
refactor: Jinja experiment for AffiliateLinks template#12776Sanket17052006 wants to merge 5 commits into
Sanket17052006 wants to merge 5 commits into
Conversation
![accesslint[bot]](https://avatars.githubusercontent.com/in/1502?s=60&v=4){kind=small}
github-project-automation
Bot
moved this to Waiting Review/Merge from Staff
in Ray's Project
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances the Phase 3 work for #12678 by introducing a Jinja2 proof-of-concept port of the AffiliateLinks macro and adding an experiment path in AffiliateLinksPartial.generate_async() to render both Templetor and Jinja2, compare outputs, and log timing/match results when enabled via env var.
Changes:
- Add a new Jinja2 template port:
openlibrary/macros/AffiliateLinks.html.jinja. - Add a canary experiment path in
openlibrary/plugins/openlibrary/partials.pyto compare Templetor vs Jinja2 output and log match/timings behindOL_JINJA_EXPERIMENT=1. - Add a new test module covering Jinja rendering and (optionally) Templetor equivalence, and introduce Jinja2 as a dependency.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| requirements.txt | Adds Jinja2 dependency for the experiment/template rendering. |
| openlibrary/macros/AffiliateLinks.html.jinja | Jinja2 port of the existing Templetor AffiliateLinks macro. |
| openlibrary/plugins/openlibrary/partials.py | Adds Jinja2 rendering helpers + experiment gating/logging in AffiliateLinksPartial.generate_async(). |
| openlibrary/plugins/openlibrary/tests/test_jinja_experiment.py | New tests for Jinja rendering and output equivalence (with optional web.py-based comparison). |
Comment on lines
+300
to
+308
| _JINJA_ENV = Environment( | ||
| loader=FileSystemLoader(Path(__file__).resolve().parent.parent.parent / "macros"), | ||
| autoescape=True, | ||
| ) | ||
| _JINJA_ENV.globals["_"] = _ | ||
|
|
||
| logger = logging.getLogger("openlibrary.partials") | ||
| _EXPERIMENT_LOGGER = logging.getLogger("openlibrary.jinja_experiment") | ||
| _JINJA_EXPERIMENT_ENABLED = os.getenv("OL_JINJA_EXPERIMENT") == "1" |
Comment on lines
+358
to
+362
| t0 = _time.perf_counter() | ||
| macro = web.template.Template.globals["macros"].AffiliateLinks(primary_stores, more_stores) | ||
| return {"partials": str(macro)} | ||
| templetor_html = str(macro) | ||
| t1 = _time.perf_counter() | ||
|
|
Comment on lines
+369
to
+382
| _EXPERIMENT_LOGGER.info( | ||
| "Jinja2 experiment | match=%s templetor_ms=%.2f jinja2_ms=%.2f | title=%s isbn=%s", | ||
| match, | ||
| (t1 - t0) * 1000, | ||
| (t3 - t2) * 1000, | ||
| title, | ||
| isbn, | ||
| ) | ||
| except Exception: | ||
| _EXPERIMENT_LOGGER.exception( | ||
| "Jinja2 experiment failed | title=%s isbn=%s", | ||
| title, | ||
| isbn, | ||
| ) |
for more information, see https://pre-commit.ci
RayBB
requested changes
May 25, 2026
Collaborator
RayBB
left a comment
RayBB
left a comment
There was a problem hiding this comment.
Great start! Can you make it so instead of rendering both for each request we accept a query parameter jinja=true and we can use that to switch between the two versions.
Also would be great if you checkout the jinja docs to ensure we follow best practices for setup here.
github-project-automation
Bot
moved this from Waiting Review/Merge from Staff
to Someone else is working on it
in Ray's Project
Contributor
Author
|
Hey @RayBB, I have switched to j |
github-actions
Bot
added
the
Needs: Response
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
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.
Closes #12678
This is the phase 3 of the #12678. It converts AffiliateLinks.html.jinja to Jinja as a proof-of-concept for Jinja in the codebase.
Technical
AffiliateLinks.html.jinjaas a direct Jinja2 port of the TempletorAffiliateLinks.htmlmacroAffiliateLinksPartial.generate_async()that renders both Templetor and Jinja2 versions, compares HTML output, and logs match status + render timesOL_JINJA_EXPERIMENT=1env var, zero overhead when disabled (returns Templetor HTML)is_botimport insidegenerate_async()Jinja2>=3.1.6torequirements.txtTesting
pytest openlibrary/plugins/openlibrary/tests/test_jinja_experiment.py -vOL_JINJA_EXPERIMENT=1tofast_webservice incompose.override.yamlcurl "http://localhost:18080/partials/AffiliateLinks.json?data=%7B%22args%22%3A%5B%22Robinson+Crusoe%22%2C%7B%22isbn%22%3Anull%2C%22asin%22%3A%22cu31924011498676%22%2C%22prices%22%3Atrue%7D%5D%7D"docker compose logs fast_web | grep "Jinja2 experiment"match=True templetor_ms=X.XX jinja2_ms=X.XXhttp://localhost:8080/books/OL24152177Mand confirm "Buy" section renders correctlyScreenshot
N/A — backend experiment only
Stakeholders
@RayBB