HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Is this an efficient way of mounting a file-system via SSH (SSHFS)?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisfilesystemefficientwaymountingsshfsviassh

Problem

#!/usr/bin/python3 

# Description: Python script that will mount FTP-Server via SSHFS protocol 
import sys
import os
import subprocess

def mount():
#Function for mounting the networked drive
    if os.path.exists('/home/user/MNT/MEDIA/'):
        print('Network drive is mounted')
        un = input("To unmount type U/u \n")
        if un == "u" or un == "U":
            subprocess.call(['sudo fusermount -u /home/user/MNT/'], shell=True)  
        else:
            pass 
        exit(0) 
    if not os.path.exists('/home/user/MNT/MEDIA/'):
        IP = input("Type I/i for local connection, E/e for external\n")
        if IP == "i" or IP == "I":
            subprocess.call(['sudo sshfs -o idmap=user -o uid=1000 -o gid=1000 -o allow_other -o default_permissions -p 100 user@10.0.0.100:/media/Hitachi/ /home/user/MNT'], shell=True)
        if IP == "e" or IP == "E":
            subprocess.call(['sudo sshfs -o idmap=user -o uid=1000 -o gid=1000 -o allow_other -o default_permissions -p 100 user@xxxxxxxx.com/media/Hitachi/ /home/user/MNT'], shell=True) 
        elif IP == "": #returns if null value 
            return mount()

def makedir():
#Function to check and see if directory is created and if not create it
    if not os.path.exists('/home/user/MNT/'):
        subprocess.call(['sudo mkdir /home/user/MNT/'], shell=True)
    if os.path.isdir('/home/user/MNT/'):
        mount() 

makedir()


EDITED:

def sshfs():
#function to mount network drive
    while True:
        user = input("Type 'i' for internal, 'e' for external connection\n")
        fs = File_System.get(user.lower()) #Get value from the dict called File_System
        if fs is None:
            print("Invalid option, enter either 'i' for internal or 'e' for external\n")
        else:
            subprocess.call(["sudo sshfs -o uid=1000 -o gid=1000 -o idmap=user -o default_permissions -o allow_other -p 100 %s %s" % (fs, Local_Mount)], shell=True)
            exit(0)

Solution

Efficiency is just not an issue in this kind of task. As "glue" code, it's not computationally intensive. There are, however, some maintainability issues.

-
In mount(), the basic outline is:

def mount():
    if os.path.exists('/home/user/MNT/MEDIA/'):
        ...
        exit(0) 
    if not os.path.exists('/home/user/MNT/MEDIA/'):
        ...


… which should be expressed as:

def mount_or_unmount():
    if os.path.exists('/home/user/MNT/MEDIA/'):
        ...
    else:
        ...


  • Using recursion for error handling is weird. That would normally be handled using a while-loop. It's also slightly odd that the empty string triggers a retry, but any other invalid input would exit the script.



-
The two sudo sshfs commands differ only slightly. Your code should be structured accordingly.

REMOTE_FS = {
    'i': 'user@10.0.0.100:/media/Hitachi/',
    'e': 'user@xxxxxxxx.com/media/Hitachi/',
}

...
while True:
    ip = input("Type I/i for local connection, E/e for external\n")
    fs = REMOTE_FS.get(ip.lower())
    if fs is None:    # Input was not 'i' or 'e'
        continue
    subprocess.call(['sudo sshfs -o idmap=user -o uid=1000 -o gid=1000 -o allow_other -o default_permissions -p 100 %s /home/user/MNT' % (fs)], shell=True)
    break

Code Snippets

def mount():
    if os.path.exists('/home/user/MNT/MEDIA/'):
        ...
        exit(0) 
    if not os.path.exists('/home/user/MNT/MEDIA/'):
        ...
def mount_or_unmount():
    if os.path.exists('/home/user/MNT/MEDIA/'):
        ...
    else:
        ...
REMOTE_FS = {
    'i': 'user@10.0.0.100:/media/Hitachi/',
    'e': 'user@xxxxxxxx.com/media/Hitachi/',
}

...
while True:
    ip = input("Type I/i for local connection, E/e for external\n")
    fs = REMOTE_FS.get(ip.lower())
    if fs is None:    # Input was not 'i' or 'e'
        continue
    subprocess.call(['sudo sshfs -o idmap=user -o uid=1000 -o gid=1000 -o allow_other -o default_permissions -p 100 %s /home/user/MNT' % (fs)], shell=True)
    break

Context

StackExchange Code Review Q#37350, answer score: 2

Revisions (0)

No revisions yet.