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

Fix resource limit scores resetting on include #1485

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

agon-qurdina
Copy link

When include is used in a template, it'll go through Liquid::Template#render, which is called on a new template instance but with the same context. Meaning the "partial" that was included will keep using & updating the same Liquid::ResourceLimits instance. Among those updates though, there is an initial, unconditional reset of the scores done inside Liquid::Template#render.

After this fix, the reset will only be applied if the method was not called inside a partial context.

@ghost ghost added the cla-needed label Oct 27, 2021
When `include` is used in a template, it'll go through
`Liquid::Template#render`, which is called on a new template
instance but with the same context. Meaning the "partial" that was
included will keep using & updating the same `Liquid::ResourceLimits`
instance. Among those updates though, there is an initial,
unconditional reset of the scores done inside `Liquid::Template#render`.

After this fix, the reset will only be applied if the method was not
called inside a partial context.
@agon-qurdina agon-qurdina force-pushed the agon-fix-resource-scores-reset-on-include branch from 2499d7a to 277a620 Compare October 27, 2021 08:28
@ghost ghost removed the cla-needed label Oct 27, 2021
@agon-qurdina agon-qurdina changed the title Fix resource limit scores resetting after include Fix resource limit scores resetting on include Nov 1, 2021
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.

None yet

1 participant