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

Checking LSF log if bjobs fails #5

Merged

Conversation

leoisl
Copy link
Collaborator

@leoisl leoisl commented Mar 14, 2020

This PR checks LSF logs if bjobs fails 3 times (by default).

In the current code, the pipeline will fail with the error message that it got an unknown job status.
In this event, this PR will look at LSF logs to check if the pipeline failed or succeeded.
However, if the logs are stored in a slow filesystem, the submission rate can degrade (my experience is that 1 job was submitted every 10-20 seconds, while when not looking at the logs in the filesystem to check the job status, the rate was several jobs per second).

Test-wise, the coverage is close to 100%. CookieCutter.py and OSLayer.py are not supposed to be tested however, they are just an abstraction on getting values from CookieCutter and interacting with the OS so that they can be mocked in the testing code.

Still searching for a way to do this without degrading the submission rate.

@mbhall88 mbhall88 self-requested a review March 15, 2020 10:22
@mbhall88 mbhall88 self-assigned this Mar 15, 2020
Copy link
Member

@mbhall88 mbhall88 left a comment

Choose a reason for hiding this comment

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

Amazing work @leoisl . The test infrastructure is very welcome and needed! This will close #1

cookiecutter.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
{{cookiecutter.profile_name}}/OSLayer.py Outdated Show resolved Hide resolved
{{cookiecutter.profile_name}}/OSLayer.py Outdated Show resolved Hide resolved
{{cookiecutter.profile_name}}/config.yaml Show resolved Hide resolved
{{cookiecutter.profile_name}}/lsf_submit.py Outdated Show resolved Hide resolved
{{cookiecutter.profile_name}}/lsf_submit.py Outdated Show resolved Hide resolved
{{cookiecutter.profile_name}}/lsf_submit.py Outdated Show resolved Hide resolved
{{cookiecutter.profile_name}}/lsf_submit.py Outdated Show resolved Hide resolved
{{cookiecutter.profile_name}}/lsf_submit.py Outdated Show resolved Hide resolved
@leoisl
Copy link
Collaborator Author

leoisl commented Mar 15, 2020

Solved the simplest changes now, will continue later

@leoisl
Copy link
Collaborator Author

leoisl commented Mar 19, 2020

I think now we have two pending items. I will now run some pipelines with the current version of the code. I wonder if we should merge into a branch while we make sure I did not introduce any bugs, and you can also work on this branch to add rule-specific cluster configs.

@mbhall88 mbhall88 changed the base branch from master to development March 20, 2020 10:50
@leoisl
Copy link
Collaborator Author

leoisl commented Mar 20, 2020

I think we are good to merge! Will keep using this profile for the next pipelines to spot any issue.

@mbhall88
Copy link
Member

I think we are good to merge! Will keep using this profile for the next pipelines to spot any issue.

Not quite. There is a conflict still. I think this is because the origin of this branch is before a change to lsf-status.py was made and now that file is deleted in this PR. We can't solve this on github, it has to be done locally.

@leoisl
Copy link
Collaborator Author

leoisl commented Mar 20, 2020

I actually tried to solve the conflict locally, but was unable to:

  1. Cherry-picking the missing commit: leoisl@12d0bde
  2. Manually adding lsf-submit.py back: leoisl@0fc5bee
    and
  3. Removing lsf-submit.py: leoisl@f3b3cf1

Will retry...

@leoisl
Copy link
Collaborator Author

leoisl commented Mar 20, 2020

So one way to be able to merge is to add the old lsf-status and lsf-submit files. We can merge and then delete them.

@mbhall88 mbhall88 merged commit 9150a8f into Snakemake-Profiles:development Mar 20, 2020
@mbhall88 mbhall88 mentioned this pull request Apr 2, 2020
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