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 flux executor support for PsijWorker #710

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

Conversation

adi611
Copy link
Contributor

@adi611 adi611 commented Oct 2, 2023

Types of changes

  • New feature (non-breaking change which adds functionality)

Summary

Adds flux executor support for PsijWorker and a GitHub Actions workflow testflux.yml for testing

Known issues

  • jobtap errors: (non-breaking; tests still pass)
jobtap: job.new: callback returned error
jobtap: job.inactive.add: callback returned error

Reference issues flux-framework/flux-core#5475 and flux-framework/flux-core#4991

  • test_task.py::test_audit_shellcommandtask_version fails - IndexError: list index out of range

@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (0e3d33c) 83.52% compared to head (993c039) 81.82%.

❗ Current head 993c039 differs from pull request most recent head 772d07a. Consider uploading reports for the commit 772d07a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #710      +/-   ##
==========================================
- Coverage   83.52%   81.82%   -1.71%     
==========================================
  Files          23       23              
  Lines        4990     4990              
  Branches     1429        0    -1429     
==========================================
- Hits         4168     4083      -85     
- Misses        814      907      +93     
+ Partials        8        0       -8     
Flag Coverage Δ
unittests 81.82% <0.00%> (-1.71%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
pydra/conftest.py 68.62% <ø> (-7.85%) ⬇️
pydra/engine/workers.py 18.31% <0.00%> (-12.15%) ⬇️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adi611
Copy link
Contributor Author

adi611 commented Oct 2, 2023

=========================== short test summary info ============================
FAILED pydra/engine/tests/test_task.py::test_audit_shellcommandtask_version - IndexError: list index out of range
= 1 failed, 1232 passed, 163 skipped, 9 xfailed, 450 warnings in 857.86s (0:14:17) =
Oct 02 18:07:18.412106 broker.err[0]: rc2.0: pytest --psij=flux --color=yes -vs --cov pydra --cov-config .coveragerc --cov-report xml:cov.xml --doctest-modules pydra/ Exited (rc=1) 858.6s

@adi611
Copy link
Contributor Author

adi611 commented Oct 3, 2023

No tests are failing now. Also, I am using psij-python from my fork of the repo here till ExaWorks/psij-python#421 is merged.

@adi611
Copy link
Contributor Author

adi611 commented Oct 21, 2023

I think this is good to go, so I'm removing the WIP tag. Please let me know if I need to make any changes.

@adi611 adi611 changed the title [WIP] Add flux executor support for PsijWorker Add flux executor support for PsijWorker Oct 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.

1 participant