aiidalab / aiidalab-widgets-base

Reusable widgets for AiiDAlab applications
MIT License
6 stars 17 forks source link

Various issues in SshComputerSetup #516

Open danielhollas opened 11 months ago

danielhollas commented 11 months ago

When looking at the code of SshComputerSetup I noticed a couple of (apparent) issues.

unkcpz commented 11 months ago

Also, the issue found in https://github.com/aiidalab/aiidalab-widgets-base/pull/511#issuecomment-1723194831

I noticed the thread created in the _on_setup_ssh_button_pressed changing the object attributes of the class, there is race condition that needs to be avoided.
The problem is that the real SSH connection part, is difficult to write unid test. Probably it is worth to try https://github.com/carletes/mock-ssh-server and http://sdf.org/

unkcpz commented 11 months ago

@yakutovicha in ssh_copy_id the function call ssh command in subprocess, maybe it is more easy to cache the output and exception with using ssh library for example paramiko? BTW, is there any progress on the ssh light of aiida-core, what is the ssh library used there?

unkcpz commented 10 months ago

When unit testing this widget, we're (repeatadly) writing to an actual .ssh/config file! We need to monkeypatch this.

Was fixed by https://github.com/aiidalab/aiidalab-widgets-base/pull/523

When a private Ssh key already exists, the new value is written to a different file, but this is not correctly propagated to the calling function

Here is a solution that is needed in my implementation that address other the private key. https://github.com/aiidalab/aiidalab-widgets-base/pull/511/commits/44fbdf64df494082ec74b8f9fa2e2ff8aa4d9099