Skip to content

Use context managers for templating changes in Bundle easyblock#3547

Merged
boegel merged 3 commits intoeasybuilders:5.0.xfrom
Flamefire:use-resolve-context-manager
Feb 26, 2025
Merged

Use context managers for templating changes in Bundle easyblock#3547
boegel merged 3 commits intoeasybuilders:5.0.xfrom
Flamefire:use-resolve-context-manager

Conversation

@Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Jan 3, 2025 •
edited by boegel
Loading

This ensures the old value is restored in all cases.

Note that there is some whitespace change which makes the diff look larger

Requires:

See also #3537 for the previous change to this code part

@boegel boegel added this to the 5.0 milestone Feb 12, 2025
@boegel boegel added change and removed enhancement labels Feb 12, 2025
boegel
boegel requested changes Feb 12, 2025
View reviewed changes
@boegel boegel mentioned this pull request Feb 12, 2025
Merged
@Flamefire Flamefire force-pushed the use-resolve-context-manager branch from f004583 to 2f114d0 Compare February 13, 2025 10:18
@Flamefire
Copy link
Contributor Author

Flamefire commented Feb 13, 2025

Found a new failure cause by resolving default_component_specs which wasn't the case before.
While fixing I found that self.cfg.update resolves the current value first and there is no way to avoid that except with the context manager. I'm not fully sure things like self.cfg.update('sources', comp_sources). I guess when adding a source using a template twice resolving would indeed at it twice while with templating disabled the duplicate would be detected.

Only question would be if we want to reduce the scope of the template-disabling and use some more for the updates. Maybe group the cfg.update statements for less clutter

@Flamefire Flamefire force-pushed the use-resolve-context-manager branch from 2f114d0 to 4788b70 Compare February 13, 2025 13:44
@Flamefire
Disable templating for more code
6a26673 
- `default_component_specs` must not be resolved
- `self.cfg.update` resolves the current value before doing the update.

Just put the whole code inside the context manager to use the old behavior
@Flamefire Flamefire force-pushed the use-resolve-context-manager branch from 4788b70 to 6a26673 Compare February 13, 2025 14:15
@Flamefire
Copy link
Contributor Author

Flamefire commented Feb 15, 2025 •
edited
Loading

Using the framework PR in a venv setup with:
pip install https://github.com/Flamefire/easybuild-framework/archive/resolve-context-manager.tar.gz https://github.com/easybuilders/easybuild-easyblocks/archive/5.0.x.tar.gz https://github.com/easybuilders/easybuild-easyconfigs/archive/5.0.x.tar.gz

Test report by @Flamefire

Overview of tested easyconfigs (in order)

Build succeeded for 15 out of 19 (16 easyconfigs in total)
i7035 - Linux Rocky Linux 8.9 (Green Obsidian), x86_64, AMD EPYC 7702 64-Core Processor, Python 3.8.17
See https://gist.github.com/Flamefire/3e7e5f4164c77e0e0e3f9555a769f9ea for a full test report.

@boegel
Copy link
Member

boegel commented Feb 18, 2025

@Flamefire What's with the failing test builds?

@Flamefire
Copy link
Contributor Author

Flamefire commented Feb 18, 2025

It's only R that failed to build due to a filesystem issue:

/usr/bin/bash: /data/horse/ws/s3248973-EasyBuild/easybuild-rome/software/R/4.2.2-foss-2022b/bin/R: /bin/sh: Defekter Interpreter: Text file busy

I'm "currently" installing R again. 811/1203 packages so far, running for 7 hrs...

@boegel
Copy link
Member

boegel commented Feb 18, 2025

It's only R that failed to build due to a filesystem issue:

/usr/bin/bash: /data/horse/ws/s3248973-EasyBuild/easybuild-rome/software/R/4.2.2-foss-2022b/bin/R: /bin/sh: Defekter Interpreter: Text file busy

I'm "currently" installing R again. 811/1203 packages so far, running for 7 hrs...

With EasyBuild 5.0dev, you should be able to use --parallel-extensions-install to speed that up significantly...
But it's far enough now that restarting it doesn't make much sense I think :)

@Flamefire
Copy link
Contributor Author

Flamefire commented Feb 18, 2025

Test report by @Flamefire

Overview of tested easyconfigs (in order)

  • SUCCESS xtensor-0.24.0-foss-2021b.eb
  • SUCCESS XML-LibXML-2.0210-GCCcore-13.2.0.eb
  • SUCCESS XML-LibXML-2.0207-GCCcore-11.3.0.eb
  • SUCCESS X11-20230603-GCCcore-12.3.0.eb
  • SUCCESS Wayland-1.22.0-GCCcore-12.3.0.eb
  • SUCCESS SentencePiece-0.2.0-GCC-13.2.0.eb
  • SUCCESS R-bundle-CRAN-2024.06-foss-2023b.eb
  • SUCCESS PyQt5-5.15.7-GCCcore-12.2.0.eb
  • SUCCESS Jupyter-bundle-20240522-GCCcore-13.2.0.eb
  • SUCCESS GTK4-4.13.1-GCC-12.3.0.eb
  • SUCCESS FlexiBLAS-3.4.4-GCC-13.3.0.eb
  • SUCCESS Autotools-20231222-GCCcore-13.3.0.eb
  • SUCCESS Autotools-20240712-GCCcore-14.2.0.eb
  • SUCCESS FlexiBLAS-3.3.1-GCC-13.2.0.eb
  • SUCCESS Xvfb-21.1.8-GCCcore-12.3.0.eb
  • SUCCESS R-4.2.2-foss-2022b.eb
  • SUCCESS arrow-R-11.0.0.3-foss-2022b-R-4.2.2.eb
  • SUCCESS R-bundle-Bioconductor-3.16-foss-2022b-R-4.2.2.eb
  • SUCCESS Seurat-4.4.0-foss-2022b-R-4.2.2.eb

Build succeeded for 19 out of 19 (16 easyconfigs in total)
i7003 - Linux Rocky Linux 8.9 (Green Obsidian), x86_64, AMD EPYC 7702 64-Core Processor (zen2), Python 3.8.17
See https://gist.github.com/Flamefire/3e2f8e80cbaf3cd250954b93086e16f0 for a full test report.

@boegel boegel changed the title Use context managers for templating changes in Bundle easyblock Use context managers for templating changes in Bundle easyblock Feb 24, 2025
@boegel
Copy link
Member

boegel commented Feb 26, 2025

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS FlexiBLAS-3.4.4-GCC-13.3.0.eb
  • SUCCESS X11-20230603-GCCcore-12.3.0.eb
  • SUCCESS foss-2023b.eb
  • SUCCESS zlib-ng-2.2.2-GCCcore-13.2.0.eb
  • SUCCESS buildenv-default-foss-2023b.eb
  • SUCCESS matplotlib-3.9.2-gfbf-2024a.eb
  • SUCCESS SentencePiece-0.2.0-GCC-13.2.0.eb
  • SUCCESS Seurat-5.1.0-foss-2023a-R-4.3.2.eb
  • SUCCESS OpenSSL-3.eb
  • SUCCESS Perl-bundle-CPAN-5.36.1-GCCcore-12.3.0.eb

Build succeeded for 10 out of 10 (10 easyconfigs in total)
node4230.shinx.os - Linux RHEL 9.4, x86_64, AMD EPYC 9654 96-Core Processor (zen4), Python 3.9.18
See https://gist.github.com/boegel/14b567da801af05890dcf285ea919db4 for a full test report.

boegel
boegel approved these changes Feb 26, 2025
View reviewed changes
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 9784237 into easybuilders:5.0.x Feb 26, 2025
19 checks passed
@Flamefire Flamefire deleted the use-resolve-context-manager branch February 27, 2025 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@boegel boegel boegel approved these changes

Assignees

No one assigned

Labels

change EasyBuild-5.0 EasyBuild 5.0

Projects

None yet

Milestone

5.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@Flamefire @boegel