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

Use Pexpect for password clusters instead of paramiko and fsspec #131

Merged
merged 11 commits into from
Nov 6, 2023

Conversation

dongreenberg
Copy link
Contributor

@dongreenberg dongreenberg commented Nov 3, 2023

Uses pexepct to direct passwords to the console for clusters which require them, eliminating fsspec and paramiko from the connection flow, and also giving us a printable string to debug password cluster connection issues with. Everything now flows through the SkyPilot SSHCommandRunner, which I've also updated our fork of.

Also adds an attempt at a simple password test suite. Tests pass for docker and cpu password clusters.

...I have no idea what's going on with the commits. Help.

- Migrate object store tests and cluster to use levels instead of ondemand_cpu_cluster
- Tests fail due to merging HTTPS changes with docker cluster changes
…g and test setup more thoroughly

- Add screen to slim dockerfiles to avoid having to use "--no-screen"
- Add default fixture levels to conftest, and module levels override
- Give keypair and pwd docker images different tags
- Migrate test_module, test_function, and test_obj_store to use `cluster` fixture. Obj store and module tests mostly pass with keypair, and fail for pwd (but pass with pexpect changes in a separate PR)
- Update SSHCommandRunner with SkyPilot's recent updates
…tore tests pass (except known bug in test_stateful_generator) with password cluster.
- Add test_pwd_cluster.py, initial concept for cluster-specific test suite.

test_pwd_cluster.py passes (other than one or two known issues) with level MINIMAL.
Copy link
Collaborator

@carolineechen carolineechen left a comment

Choose a reason for hiding this comment

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

super cool!

runhouse/resources/hardware/cluster.py Show resolved Hide resolved
runhouse/resources/hardware/utils.py Show resolved Hide resolved
…g and test setup more thoroughly

- Add screen to slim dockerfiles to avoid having to use "--no-screen"
- Add default fixture levels to conftest, and module levels override
- Give keypair and pwd docker images different tags
- Migrate test_module, test_function, and test_obj_store to use `cluster` fixture. Obj store and module tests mostly pass with keypair, and fail for pwd (but pass with pexpect changes in a separate PR)
- Add test_pwd_cluster.py, initial concept for cluster-specific test suite.

test_pwd_cluster.py passes (other than one or two known issues) with level MINIMAL.
@dongreenberg dongreenberg merged commit 6a2be29 into main Nov 6, 2023
@dongreenberg dongreenberg deleted the pexpect branch November 6, 2023 21:42
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.

2 participants