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

parsl.providers.cluster_provider _write_submit_script should return nothing, rather than constant True #3239

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mirembe-mariam
Copy link

Description

parsl.providers.cluster_provider _write_submit_script should return nothing, rather than constant True

Changed Behaviour

Changed the return handling for the cluster_provider _write_submit_script and edited the docstring to match

Fixes

Fixes # issue #3234

Type of change

  • Bug fix
  • New feature
  • Update to human readable text: Documentation/error messages/comments
  • Code maintenance/cleanup

@benclifford
Copy link
Collaborator

Hi. Your tests were failing - see the red x next to one of the checks on this PR. The error it reports (which you can find by clicking Details) is:

flake8 parsl/
parsl/providers/cluster_provider.py:94:21: W291 trailing whitespace
parsl/providers/cluster_provider.py:120:15: W291 trailing whitespace
make: *** [Makefile:40: flake8] Error 1
Error: Process completed with exit code 2.

PR #3238 also makes this change and is merged now, so you can fix this PR to make the tests pass, if you would like to see how to fix things, or you can move on to doing something else and close this PR without merging.

@mirembe-mariam
Copy link
Author

Hi. Your tests were failing - see the red x next to one of the checks on this PR. The error it reports (which you can find by clicking Details) is:

flake8 parsl/
parsl/providers/cluster_provider.py:94:21: W291 trailing whitespace
parsl/providers/cluster_provider.py:120:15: W291 trailing whitespace
make: *** [Makefile:40: flake8] Error 1
Error: Process completed with exit code 2.

PR #3238 also makes this change and is merged now, so you can fix this PR to make the tests pass, if you would like to see how to fix things, or you can move on to doing something else and close this PR without merging.

Alright let me fix it

@mirembe-mariam
Copy link
Author

Hi, @benclifford I have been able to resolve the issues with the failing tests.

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.

None yet

2 participants