Skip to content

Conversation

@Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Jan 3, 2025

Both (templating and disabling the enforcing) should only be disabled temporarily so make the state private and expose only the context manager and a property.

2 small fixes in adjacent code:

  • Use a try-except instead of an if-else (avoids an extra lookup)
  • Use the resolve_template member instead of the free function to remove some C&P code

I also intended to make asdict trivial (see 2a63863) but as __getitem__ checks for deprecated parameters on each access it might impact performance too much.

Improvement after #4726

@Flamefire Flamefire force-pushed the resolve-context-manager branch from 1c7bdec to 45196fb Compare January 3, 2025 14:48
@boegel boegel added the EasyBuild-5.0 EasyBuild 5.0 label Feb 12, 2025
@boegel boegel added this to the 5.0 milestone Feb 12, 2025
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm, but I would like to see this tested thoroughly via updated Bundle easyblock in easybuilders/easybuild-easyblocks#3547 before merging it...

@Flamefire
Copy link
Contributor Author

I can do that. Any specific ECs in mind or just 3-4 ones using easblock = 'Bundle'?

@Flamefire
Copy link
Contributor Author

@boegel Test report using this: easybuilders/easybuild-easyblocks#3547 (comment)

…ther than accessing private class variables directly
@boegel boegel added the change label Feb 24, 2025
@boegel boegel changed the title Add context manager for allowing unresolved templates and make the state members private Add context manager for allowing unresolved templates and make the state members private + remove support for directly setting enable_templating and expect_resolved_template_values Feb 24, 2025
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel merged commit 01b1bfd into easybuilders:5.0.x Feb 26, 2025
39 checks passed
@Flamefire Flamefire deleted the resolve-context-manager branch February 27, 2025 07:44
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.

2 participants