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.

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

Add SLES15 as a platform #354

Merged
merged 21 commits into from
Jun 28, 2024
Merged

Add SLES15 as a platform #354

merged 21 commits into from
Jun 28, 2024

Conversation

Dooruk
Copy link
Collaborator

@Dooruk Dooruk commented Jun 13, 2024

It is finally happening....

... this will require some changes in the .swell and .cylc folders.

@Dooruk Dooruk linked an issue Jun 13, 2024 that may be closed by this pull request
2 tasks
@Dooruk Dooruk requested a review from ashiklom June 17, 2024 21:05
@Dooruk
Copy link
Collaborator Author

Dooruk commented Jun 20, 2024

I'm proposing not using .swell/slurm-swell.yaml and moving it under platforms (which is already done but I forgot to explain my rationale), so the user would need to use the -s option unless we set the account etc. NCCS Discover generic.

Just need to fix the mock test next..

@Dooruk Dooruk marked this pull request as ready for review June 24, 2024 20:01
@Dooruk
Copy link
Collaborator Author

Dooruk commented Jun 25, 2024

@ashiklom this is ready for a review. I commented out user_globals, so I'm proposing having global_defaults in platforms, nothing in the $HOME directory (now that we have them in platforms, no need for user_globals), with the -s option for the user during swell create. Hence, I had to comment out lines from the code_tests please take a look when you have a chance.

I had to introduce platform in both code_test and the slurm utility but we might need a more elegant solution..

@Dooruk Dooruk requested a review from ashiklom June 26, 2024 14:16
@Dooruk
Copy link
Collaborator Author

Dooruk commented Jun 26, 2024

I had to add a unit_test switch to prepare_scheduling_dict to allow platform catch conditional to work but looks good on my end.

@Dooruk
Copy link
Collaborator Author

Dooruk commented Jun 27, 2024

Can I get that approval now 😄

@Dooruk
Copy link
Collaborator Author

Dooruk commented Jun 27, 2024

Hmm, since we are not using the global swell-slurm.yaml anymore, gmao_ci automatically uses the default settings. That is one minor issue but that's related to how our Github actions are setup currently.

But thinking about it further, this will impact other users too, unless they include this in their -s slurm-override.yaml.

@ashiklom
Copy link
Collaborator

Hmm, since we are not using the global swell-slurm.yaml anymore, gmao_ci automatically uses the default settings. That is one minor issue but that's related to how our Github actions are setup currently.

But thinking about it further, this will impact other users too, unless they include this in their -s slurm-override.yaml.

Yes, removing the user_globals / ~/.swell/swell-slurm.yaml is a breaking change, and not necessarily a necessary one for this feature. I think keeping user_globals around is safer --- there are platform-agnostic but still useful ways to set the user-specific defaults (most importantly, setting your user account), and it can always be overridden by -s slurm.yaml, so it's easy to soft-deprecate it gradually (rather than ripping it out abruptly). It's also consistent with how other SLURM-related software does things; e.g., Dask jobqueue.

Personally, my vote would be to bring user_globals back. But if you decide to leave it out, a simple fix to the CI would be to add -s /home/gmao_ci/.swell/swell-slurm.yaml to every swell create command in the CI.

@Dooruk
Copy link
Collaborator Author

Dooruk commented Jun 28, 2024

Personally, my vote would be to bring user_globals back. But if you decide to leave it out, a simple fix to the CI would be to add -s /home/gmao_ci/.swell/swell-slurm.yaml to every swell create command in the CI.

Ok, I revived user_globals and now we have not just one but two mock tests!

@Dooruk Dooruk merged commit d9410c5 into develop Jun 28, 2024
11 checks passed
@Dooruk Dooruk deleted the feature/sles15_modules branch June 28, 2024 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SLES15 as a platform
2 participants