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

Samba Access Wrapper

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

Problem

I created a Python module to access Samba shares some time ago, using Mike Teos' SMB module as some kind of a base to it. It is mostly a wrapper, but as I said, I wrote it some time ago and would appreciate feedback on readability, usability and modularity of this module.

ServerAccess.py

```
#!/usr/bin/env python2
# -- coding: utf-8 --
__DEBUG__=9

from smb.SMBConnection import SMBConnection
from smb.base import SharedDevice
from smb.base import SharedFile

import tempfile
from socket import getfqdn
from Networking import resolveNetworkTuple

import logging
import sys
import os # Path operations

class ServerException(Exception):
pass

class ServerAccess:
"""
Description: ServerAccess Class and File Handler
"""
def __init__(self,username, password, client_name=None, server_name=None, server_ip=None, shared_folder=None, timeout=5):
self.__logger=logging.getLogger("Notenverwaltung")
# use_ntlm_v2 must be passed as named argument
self.__logger.debug(" username = %s"%(username))
self.__logger.debug(" password = xxxxxxxxxx")
self.__logger.debug(" client_name = %s"%(client_name))
self.__logger.debug(" server_name = %s"%(server_name))
self.__logger.debug(" server_ip = %s"%(server_ip))
self.__logger.debug(" shared_folder = %s"%(shared_folder))
self.__logger.debug(" timeout = %s"%(timeout))

self.__user=username
self.__pass=password

if client_name==None:
client_name=getfqdn()
self.__client=client_name
if server_ip:
server_ip, server_name=resolveNetworkTuple("%s"%server_ip)
elif server_name:
server_ip, server_name=resolveNetworkTuple("%s"%server_name)
self.__serv

Solution

Please stick to 4 spaces for indentation. It's the Python standard and makes things much easier to read. You can read about this and many other Python style notes in the official style guide, I highly recommend it for readability.

Instead of == None it's more Pythonic to use is None.

You're also using the old % string formatting, use the newer one instead as it makes things easier and more readable. The new way is str.format, here's an example:

self.__logger.debug("Constructor connects to: \n%s"%self)


turns into

self.__logger.debug("Constructor connects to: \n{}".format(self))


Those {} get replaced by the parameters passed to format. It might not seem much use in this case, but later when you have multiple parameters it'll be much better, so it's good to be used to it.

It's great to see a __str__, implementing them is good practice. You can make your life a whole lot easier though by using implicit string concatenation. If two string literals are placed side by side with nothing but space between them, Python just concatenates them observe:

>>> "concat" "enation"
'concatenation'
>>> "concat"            "enation"
'concatenation'


This can work over multiple lines, if you put parentheses around the full expression:

>>> ("concat"
 "enation")
'concatenation'
>>> ("concat"
 "ena"
     "tio"
 "n")
'concatenation'


Of course this can make your result making much easier:

def __str__(self):
    result = ("\n"
              "\n   %s"%self.__user
              "\n   xxxxxx"
              "\n   %s"%self.__client
              "\n   %s"%self.__server
              "\n   %s"%self.__serverIP
              "\n   %s"%self.__port)
    if self.shared:
        result+=("\n   %s"%self.shared)
    result += ("\n   %s"%self.__timeout
               "\n")
    return result


Avoid using bare try excepts! You do it a few times. This will ignore any exceptions that arise. Even something like a SystemExit or a KeyboardInterrupt. You should at least use except Exception: in order to ignore those two cases. It would also be a good idea to log what exceptions occurred, as it could have been one you weren't anticipating. If you don't log unanticipated exceptions then you'll never know what they are, nor will you know how to fix the bug causing them.

You should always put import statements at the top of the file, so that people know where to look for them when they see you using them. Otherwise people will get confused when they scroll to the top and don't see them anywhere.

Code Snippets

self.__logger.debug("Constructor connects to: \n%s"%self)
self.__logger.debug("Constructor connects to: \n{}".format(self))
>>> "concat" "enation"
'concatenation'
>>> "concat"            "enation"
'concatenation'
>>> ("concat"
 "enation")
'concatenation'
>>> ("concat"
 "ena"
     "tio"
 "n")
'concatenation'
def __str__(self):
    result = ("\n"
              "\n   %s"%self.__user
              "\n   xxxxxx"
              "\n   %s"%self.__client
              "\n   %s"%self.__server
              "\n   %s"%self.__serverIP
              "\n   %s"%self.__port)
    if self.shared:
        result+=("\n   %s"%self.shared)
    result += ("\n   %s"%self.__timeout
               "\n")
    return result

Context

StackExchange Code Review Q#110810, answer score: 3

Revisions (0)

No revisions yet.