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

fix: launch_cmd not set when provided to FluxExecutor #2634

Merged
merged 1 commit into from Mar 21, 2023

Conversation

vsoch
Copy link
Contributor

@vsoch vsoch commented Mar 17, 2023

Problem: Currently, the launch_cmd is not set if provided to the FluxExecutor directly. Many jobs are likely to start already with access to a flux instance, in which case the launch command should use flux submit instead of flux start.
Solution: Ensure the launch_cmd is set.

Description

Please include a summary of the change and (optionally) which issue is fixed. Please also include
relevant motivation and context.

Fixes #2633

Type of change

Choose which options apply, and delete the ones which do not apply.

  • Bug fix (non-breaking change that fixes an issue)

@vsoch
Copy link
Contributor Author

vsoch commented Mar 17, 2023

Looks like an issue with pytest versioning:

ImportError: cannot import name 'Config' from 'pytest' (/opt/hostedtoolcache/Python/3.10.10/x64/lib/python3.10/site-packages/pytest/__init__.py)
make: *** [Makefile:53: local_thread_test] Error 1
Error: Process completed with exit code 2.

I'll await further instruction, since this is out of scope for my PR. Gnite!

@benclifford
Copy link
Collaborator

Yeah there's a separate PR open to fix that, which should get merged in the next few days

Copy link
Contributor

@jameshcorbett jameshcorbett left a comment

Choose a reason for hiding this comment

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

LGTM. Definitely a bug, good catch!

@vsoch
Copy link
Contributor Author

vsoch commented Mar 20, 2023

Thanks @jameshcorbett !

@benclifford
Copy link
Collaborator

@vsoch wrt ImportError: cannot import name 'Config' from 'pytest', I merged the fix for this in #2630. Can you update your branch to latest master? (or change permissions so that I can?)

Problem: Currently, the launch_cmd is not set if provided to the
FluxExecutor directly. Many jobs are likely to start
already with access to a flux instance, in which case
the launch command should use flux submit instead of
flux start.
Solution: Ensure the launch_cmd is set.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch vsoch force-pushed the fix/flux-executor-launch-cmd branch from c8d418c to 8d1f09c Compare March 21, 2023 13:27
@vsoch
Copy link
Contributor Author

vsoch commented Mar 21, 2023

All set for both.

@benclifford benclifford merged commit 5f2e7e1 into Parsl:master Mar 21, 2023
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.

Suggestion: use equivalent of mkdir -p with runinfo
3 participants