AgnostiqHQ / covalent-slurm-plugin

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

Make `ssh_key_file` optional in `asyncssh.connect` since, by default, `asyncssh` tries many common options #79

Closed Andrew-S-Rosen closed 1 year ago

Andrew-S-Rosen commented 1 year ago

The asyncssh.connect documentation states the following:

If no client keys are specified,
an attempt will be made to load them from the files
:file:`.ssh/id_ed25519_sk`, :file:`.ssh/id_ecdsa_sk`,
:file:`.ssh/id_ed448`, :file:`.ssh/id_ed25519`,
:file:`.ssh/id_ecdsa`, :file:`.ssh/id_rsa`, and
:file:`.ssh/id_dsa` in the user's home directory, with
optional certificates loaded from the files
:file:`.ssh/id_ed25519_sk-cert.pub`,
:file:`.ssh/id_ecdsa_sk-cert.pub`, :file:`.ssh/id_ed448-cert.pub`,
:file:`.ssh/id_ed25519-cert.pub`, :file:`.ssh/id_ecdsa-cert.pub`,
:file:`.ssh/id_rsa-cert.pub`, and :file:`.ssh/id_dsa-cert.pub`.

As such, I think it would be nice to allow the user to take advantage of this flexibility and not provide an ssh_key_file if they want one of these defaults to be selected.

The lines impacted by this PR weren't tested. I'll need to come up with some creative monkeypatching in order to make codecov happy, I think.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 20.00% and project coverage change: -0.65% :warning:

Comparison is base (47479a8) 85.76% compared to head (7569e4f) 85.11%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #79 +/- ## =========================================== - Coverage 85.76% 85.11% -0.65% =========================================== Files 2 2 Lines 309 309 =========================================== - Hits 265 263 -2 - Misses 44 46 +2 ``` | [Files Changed](https://app.codecov.io/gh/AgnostiqHQ/covalent-slurm-plugin/pull/79?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/79?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ#diff-Y292YWxlbnRfc2x1cm1fcGx1Z2luL3NsdXJtLnB5) | `85.06% <20.00%> (-0.65%)` | :arrow_down: |

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

Andrew-S-Rosen commented 1 year ago

I'll re-open this in a bit with a slightly modified approach.