Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

internal/config: Remove redundant context append for parsing #4744

Merged
merged 3 commits into from
May 25, 2023

Conversation

briancain
Copy link
Member

Prior to this commit, if a config had many pipeline stanzas, say more than 6, each time we would parse a pipeline via the hcl EvalContext we would append that context to the current one to capture any additional parts of the config. Given that we are evaluating the entire config file and not parts of it, this action was redundant. Additionally, it made parsing many pipeline configs slow for each appended context.

This fixes it by removing the append. We don't need to append this, given we have the full config context each time we parse pipelines.

Thank you @jgwhite and @sdav9375 for identifying this!

Prior to this commit, if a config had many pipeline stanzas, say more
than 6, each time we would parse a pipeline via the hcl EvalContext we
would append that context to the current one to capture any additional
parts of the config. Given that we are evaluating the entire config file
and not parts of it, this action was redundant. Additionally, it made
parsing many pipeline configs slow for each appended context.

This fixes it by removing the append. We don't need to append this,
given we have the full config context each time we parse pipelines.
@briancain briancain added pr/no-changelog No automatic changelog entry required for this pull request backport/0.11.x labels May 25, 2023
@briancain briancain requested a review from a team as a code owner May 25, 2023 16:27
@github-actions github-actions bot added the core label May 25, 2023
@briancain briancain removed the pr/no-changelog No automatic changelog entry required for this pull request label May 25, 2023
jgwhite
jgwhite previously approved these changes May 25, 2023
@jgwhite jgwhite dismissed their stale review May 25, 2023 16:49

Jumped the gun — there are still test failures.

@briancain briancain merged commit 1165f11 into main May 25, 2023
39 checks passed
@briancain briancain deleted the bug/pipelines/dont-append-same-context branch May 25, 2023 20:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants