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.

Sign up for GitHub

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

Jump to bottom

Replace parallel easyconfig parameter by maxparallel #4398

Closed
wants to merge 5 commits into from
Closed

Replace parallel easyconfig parameter by maxparallel #4398

wants to merge 5 commits into from

Conversation

Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Dec 7, 2023 •
edited
Loading

Continuation of #3842 for 5.0 to fully remove the parallel EC parameter

Requires:

It is on top of the deprecating PR above fully removing the support (for after deprecation)

It requires that some easyblocks are updated first.

@Flamefire Flamefire mentioned this pull request Dec 7, 2023
Merged
@boegel boegel added the EasyBuild-5.0 EasyBuild 5.0 label Dec 20, 2023
@boegel boegel added this to the 5.0 milestone Dec 20, 2023
@boegel boegel changed the title Replace parallel EC parameter by maxparallel Replace parallel easyconfig parameter by maxparallel Jan 3, 2024
@Flamefire Flamefire force-pushed the deprecate_parallel-5.0 branch from d2ef1e7 to 91ebd42 Compare April 16, 2024 07:48
@boegel boegel added the EasyBuild-5.0-blocker Blocker for EasyBuild 5.0 label Jun 5, 2024
@Flamefire Flamefire force-pushed the deprecate_parallel-5.0 branch from 91ebd42 to 0c58470 Compare June 6, 2024 11:51
@Flamefire Flamefire force-pushed the deprecate_parallel-5.0 branch from 0c58470 to af62d8e Compare August 8, 2024 07:47
@Flamefire Flamefire force-pushed the deprecate_parallel-5.0 branch 2 times, most recently from a793f08 to 6dbe018 Compare September 17, 2024 10:28
@boegel
Copy link
Member

boegel commented Oct 8, 2024

@Flamefire Not supporting the parallel easyconfig parameter at all anymore is a bridge too far imho, even for EasyBuild 5.0, especially since it's a commonly used easyconfig parameter (as shown in easybuilders/easybuild-easyconfigs#19375).

So I think we should proceed with #4580 instead, and only actually remove support for using the parallel easyconfig parameter in the next major EasyBuild release (6.0).

So this PR should probably be closed, I'll look into reviewing #4580 ASAP...

@Flamefire
Copy link
Contributor Author

Agreed, especially as this is basically #4580 with an extra commit which we can apply at any later point. The EC PR is still useful to promote the use of the new name.

@Flamefire
Deprecate use of ec['parallel'] and fix updating the template value
862e797 
ec['parallel'] currently doubles as an EC option and as the storage for
the calculated parallelism set by the EasyBlock.
This makes it hard to reason about especially as maxparallel has pretty
much the same effect. Also changes to ec['parallel'] done by e.g.
easyblocks (or the set_parallel method) are not reflected by the
template `%(parallel)s`

Solution: Introduce a property which on write updates the template and
some magic to mirror the effect of the now deprecated ec['parallel']
@Flamefire
Update use of parallel in parallel extension installation
113820a 
Migrate from `self.cfg['parallel']` to `self.cfg.parallel`
@Flamefire
Move deprecation to 5.1
d2abfbb 
We need to fix our own easyblocks first but that requires this change.
So allow until 5.1 or remove after updating the easyblocks.
@Flamefire
Replace parallel->maxparallel in test ECs
162621f 
The EC parameter is deprecated so the tests fail.
@Flamefire
Remove _parallelLegacy
f7ce246
@Flamefire Flamefire force-pushed the deprecate_parallel-5.0 branch from 6dbe018 to f7ce246 Compare December 4, 2024 08:53
@boegel boegel requested a review from Micket January 8, 2025 15:50
@boegel
Copy link
Member

boegel commented Jan 8, 2025

Hard replacing of parallel is a bridge too far, we'll go with the deprecation approach being implemented in #4580, so closing this...

Hard removal of support for parallel easyconfig parameter should only be done in EasyBuild v6.0, which is years away...

@boegel boegel closed this Jan 8, 2025
@Flamefire
Copy link
Contributor Author

To me it doesn't look like a bad idea because the "fix" for affected files is trivial. No behavior change, just a name change.
However I do agree that a deprecation period is better for now.

@Flamefire Flamefire deleted the deprecate_parallel-5.0 branch January 8, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Micket Micket Awaiting requested review from Micket

Assignees
No one assigned
Labels
EasyBuild-5.0-blocker Blocker for EasyBuild 5.0 EasyBuild-5.0 EasyBuild 5.0
Projects
None yet
Milestone
5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Flamefire @boegel