Skip to content

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

aakash-test7
Copy link

@aakash-test7 aakash-test7 commented Mar 3, 2025

@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.

@aakash-test7
Copy link
Author

As per your suggestions, the changes will be done as soon as possible .

@JoeZiminski
Copy link
Member

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.

@JoeZiminski JoeZiminski changed the title Fix for issue #449 Check central path is writeable during SSH setup (#449) Apr 30, 2025
@JoeZiminski
Copy link
Member

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)

@aakash-test7
Copy link
Author

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!

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