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

Make shell and slurm scheduler report crashes #13

Merged
merged 14 commits into from
Jan 17, 2025

Conversation

BerengerBerthoul
Copy link
Member

@BerengerBerthoul BerengerBerthoul commented Jan 8, 2025

Actually contains:

  • Functionality to make shell and slurm schedulers report crashes
  • An updated documentation (in particular with the description of the shell and slurm schedulers)
  • Some minor refactorings here and there
  • Repair of the Windows CI
    Solves shell scheduler : need to report when test crash #11

@BerengerBerthoul BerengerBerthoul self-assigned this Jan 8, 2025
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
pytest_parallel/plugin.py Outdated Show resolved Hide resolved
pytest_parallel/plugin.py Outdated Show resolved Hide resolved
pytest_parallel/plugin.py Outdated Show resolved Hide resolved
Copy link
Contributor

@couletj couletj left a comment

Choose a reason for hiding this comment

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

Hello, j'ai fais le tour avec qq suggestions pour la doc et dans le code. Je pense que j'ai a peu près compris les idées, l'implémentation est pas évidente car beaucoup de concept et de framework utilisés (MPI, pytest, sockets, slurm, filesystem ...) mais le code est plutot lisible donc bien joué, ça devait pas être évident de rendre ça lisible.

Quelques notes générales :

  • Je pense qu'on pourrait enlever la dépendance à numpy, c'est utilisé à deux endroits dont 1 assert
  • Est ce que le choix du scheduler seq. par défaut est vraiement pertinent ? Puisqu'il est la seulement pour débuger, il faudrait peut etre mettre le statique à la place non ?
  • Est ce qu'on incrémente le num de version suite à ce dev?

pytest_parallel/shell_static_scheduler.py Outdated Show resolved Hide resolved
pytest_parallel/shell_static_scheduler.py Outdated Show resolved Hide resolved
pytest_parallel/send_report.py Outdated Show resolved Hide resolved
tracepath utility does not have a '-4' option on older versions,
so remove it since it is the default anyway
@BerengerBerthoul BerengerBerthoul force-pushed the dev_report_crash branch 3 times, most recently from b804732 to fd89241 Compare January 17, 2025 15:55
@BerengerBerthoul BerengerBerthoul force-pushed the dev_report_crash branch 2 times, most recently from ef54118 to 4508055 Compare January 17, 2025 17:29
@BerengerBerthoul
Copy link
Member Author

Thanks for the review. I think I took into account all the suggestions.

The only thing that I currently can't change is the default scheduler. The dynamic one should be the default, but it fails for some environments, see #14)

@BerengerBerthoul BerengerBerthoul merged commit f37989c into master Jan 17, 2025
42 checks passed
@BerengerBerthoul BerengerBerthoul deleted the dev_report_crash branch January 17, 2025 22:44
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.

2 participants