-
Notifications
You must be signed in to change notification settings - Fork 22
Check central path is writeable during SSH setup (#449) #473
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
base: main
Are you sure you want to change the base?
Conversation
As per your suggestions, the changes will be done as soon as possible . |
Hi @aakash-test7 thanks for this suggestion, are you still interested in working on this PR? I'm happy to help get it over the line if useful, thanks a lot. |
I renamed the PR, just as it can be useful to provide a description so the aim is clear from reading (without having to refer to the issue) |
Hi @JoeZiminski — thanks for checking in! Finished a couple of assignment projects and now will get back to finish this one. I’ve refactored the check_write_permission into ssh.py and am now focusing on combining it with run_ssh_command, while ensuring the connection is established early and logging is turned off as discussed. I appreciate your offer to help — I’ll reach out if I run into any issues. Thanks again for the feedback and support! |
@JoeZiminski
This is a nice idea, but taking an input here would freeze the application when run through the TUI. At this stage we can trust the user has input the path correctly at the config stage (if they have not, they will realize after a permission error below). Here I would delete the input code and use self.cfg["central_path"] directly in self._check_write_permissions
Following this, in datasshuttle_class.py removed input prompt .
Then
This is cool, I think this function can go into ssh.py and be merged with run_ssh_command. It is nice to encapsulate the code for writing commands over SSH but as we do this [elsewhere](https://github.com/neuroinformatics-unit/datashuttle/blob/166f8620199b6392df43d0889a8b5e34b263d3a0/datashuttle/utils/ssh.py#L66) in a way that would make it difficult to use run_ssh_command. For now we can use client.exec_command more flexibly from within other functions and refactor to centralise under a single function at a later time
I refactored this to move the check_write_permission function to ssh.py . Now, I am trying to focus on further narrow it down to establish complete connection early and combining check_write_permission with run_ssh_command . Where logging will be set to off.