search

AgnostiqHQ / covalent-slurm-plugin

Executor plugin interfacing Covalent with Slurm
https://covalent.xyz
Apache License 2.0
27 stars 6 forks source link

Remove sshproxy utility #78

Closed Andrew-S-Rosen closed 11 months ago

Andrew-S-Rosen commented 12 months ago

This PR is perhaps a bit contentious, but I think since it's worth removing here because: a) it's possible to submit a NERSC job using a certificate file (extensions for certificate timeouts can be requested); b) it's possible to use the newly developed sfapi plugin for long-term use; c) the relevant sshproxy code is largely untested.

This would make #66 obsolete.

Or we can move it to its own branch?

codecov[bot] commented 12 months ago

Codecov Report

Patch coverage has no change and project coverage change: +7.35% :tada:

Comparison is base (47479a8) 85.76% compared to head (e0db62a) 93.11%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #78 +/- ## =========================================== + Coverage 85.76% 93.11% +7.35% =========================================== Files 2 2 Lines 309 276 -33 =========================================== - Hits 265 257 -8 + Misses 44 19 -25 ``` | [Files Changed](https://app.codecov.io/gh/AgnostiqHQ/covalent-slurm-plugin/pull/78?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) | Coverage Δ | | |---|---|---| | [covalent\_slurm\_plugin/slurm.py](https://app.codecov.io/gh/AgnostiqHQ/covalent-slurm-plugin/pull/78?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ#diff-Y292YWxlbnRfc2x1cm1fcGx1Z2luL3NsdXJtLnB5) | `93.09% <ø> (+7.37%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wjcunningham7 commented 11 months ago
  • [x] I have added the tests to cover my changes.
  • [x] I have updated the documentation, VERSION, and CHANGELOG accordingly.
  • [x] I have read the CONTRIBUTING document.

This PR is perhaps a bit contentious, but I think since it's worth removing here because: a) it's possible to submit a NERSC job using a certificate file (extensions for certificate timeouts can be requested); b) it's possible to use the newly developed sfapi plugin for long-term use; c) the relevant sshproxy code is largely untested.

This would make #66 obsolete.

Or we can move it to its own branch?

This works for me! Thanks @arosen93.

CC @tylern4 @lastephey