Skip to content

Introduce contextmanager for disabling templating and reduce resolving errors#3287

Merged
akesandgren merged 2 commits intoeasybuilders:developfrom
Flamefire:less_templating
May 1, 2020
Merged

Introduce contextmanager for disabling templating and reduce resolving errors#3287
akesandgren merged 2 commits intoeasybuilders:developfrom
Flamefire:less_templating

Conversation

@Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Apr 17, 2020

The context manager is a better alternative to the currently widely used pattern of

old = value
value = False
# do work
value = old

Advantage: Shorter and safer as the reset cannot be forgotten even in case of exceptions

Afterwards some template resolving failures are removed which are observed in #3285

Diff looks larger than it is as indentation has changed and Github seems to be bad in handling this. All changes are trivial.

@smoors smoors changed the title Introduce contextmanager for disablingt templating and reduce resolving errors Introduce contextmanager for disabling templating and reduce resolving errors Apr 18, 2020
@smoors smoors added the change label Apr 18, 2020
@easybuilders easybuilders deleted a comment from boegelbot May 1, 2020
@boegel boegel added this to the next release (4.2.1?) milestone May 1, 2020
for key in self.options:
for key, value in self.options.items():
if key in self.cfg:
self.cfg[key] = resolve_template(self.options[key], self.cfg.template_values)
Copy link
Member

Choose a reason for hiding this comment

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

This was a bit silly indeed, since templates in self.options are already resolved above... Nice catch!

Copy link
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren
Copy link
Contributor

Going in, thanks @Flamefire!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants