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

Improvements for part.full_name #5946

Merged

Conversation

SchrodingersGat
Copy link
Member

  • Compile and cache the template
  • Reduces typical render time from ~20ms to ~0.2ms

Closes #5942

- Compile and cache the template
- Reduces typical render time from ~20ms to ~0.2ms
@SchrodingersGat SchrodingersGat added this to the 0.13.0 milestone Nov 20, 2023
Copy link

netlify bot commented Nov 20, 2023

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit 9143743
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/655b551ab567520008899b31


import logging

from jinja2 import Environment
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Is there a performance reason you are using Jinja2 instead of Djangos Template engine (which is based on Jinja2)? This will mean the syntax between reports and this field is different. I have not seen significant timing differences between Jinja2 and djangos template language when cached.

Copy link
Member Author

Choose a reason for hiding this comment

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

@matmair this is specifically not to break current functionality. We have been using jinja2 here for a long time - although I cannot remember why.

However changing the rendering engine at this stage would break the current default template string - jinja2 supports some tag attributes which the Django engine does not

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd be happy to look at switching to Django template engine in a separate PR

@SchrodingersGat SchrodingersGat merged commit 317c266 into inventree:master Nov 20, 2023
24 checks passed
@SchrodingersGat SchrodingersGat deleted the full-name-template-fix branch November 20, 2023 13:12
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.

[Review] "full_name" for Part
2 participants