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.

Sign up for GitHub

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

Jump to bottom

fix gotocomputer when key_filename missing #3593

Merged
merged 3 commits into from
Dec 2, 2019
Merged

fix gotocomputer when key_filename missing #3593

merged 3 commits into from
Dec 2, 2019

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Nov 30, 2019 •
edited
Loading

fix #3588
fix #3401

  • the "verdi calcjob gotocomputer" command was broken when used on computers that were configured without a value for the ssh key_filename.
  • "verdi calcjob gotocomputer" obviously does not work when no authinfo is
    present for the computer to go to. There was a bug in printing the exception, resulting in an exception bubbling up to user space (instead of a critical error with a helpful message).

@ltalirz
fix gotocomputer when key_filename missing
d6672e2 
the "verdi calcjob gotocomputer" command was broken when used on
computers that were configured without a value for the ssh key_filename.
@ltalirz
fix print of exception in gotocomputer
3a6499c 
"verdi calcjob gotocomputer" obviously does not work when no authinfo is
present for the computer to go to.
There was a bug in printing the exception, resulting in an exception
bubbling up to user space (instead of a critical error with a helpful
message).

The error message now looks like this:

$ verdi -p quicksetup calcjob gotocomputer 712
Critical: NotExistent('User<daniele.ongari@epfl.ch> has no configuration for Computer<fidis (Imported #0)>')
@ltalirz
Copy link
Member Author

ltalirz commented Nov 30, 2019 •
edited
Loading

I noticed that there does not seem to be any test of the gotocomputer command.
Perhaps not that easy to test but I guess also not impossible. @giovannipizzi Perhaps you could open an issue with some pointers on where this test should go / existing tests one could borrow code from.

@giovannipizzi
Copy link
Member

It would be great to have a test for this, but I'm not sure how... One should have a configured computer and a calculation run in there (one could use the test_daemon.py file that actually runs simulations, but I'm not sure this is over SSH - actually maybe @sphuber is changing to local in #3571?). But then one needs to test if an interactive shell is opened... Not sure how to do it exactly.

Maybe one could instead just get internally the command that is generated ssh ... and append something execute (e.g. create an folder with a given file in /tmp/XXX, then append ls /tmp/XXX to the ssh command, and check if the result shows correctly the folder content? Even if I'm not sure how robust this test would be).

@ltalirz ltalirz merged commit 67a1546 into aiidateam:develop Dec 2, 2019
@sphuber sphuber deleted the issue_3588_calcjob_gotocomputer branch December 2, 2019 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giovannipizzi giovannipizzi giovannipizzi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@ltalirz @giovannipizzi @sphuber