3
\$\begingroup\$

Here is the script that I would like reviewed for the following:

  1. Best practices and design pattern usage
  2. Correctness in unanticipated cases

The script is made to store (IPFS) Interplanetary File System hash objects. If you would like to know more check out https://ipfs.io

3rd party packages:

Script:

#!/usr/local/bin/python3

import os
import gnupg
import ipfsapi

# Set GPG Home directory
gpg = gnupg.GPG(homedir='')
fileToEncrypt = input("Full path to file to encrypted and posted \n:")
encryptedFile = (fileToEncrypt + ".gpg")

def encryptFileFunction():
    passphrase = input("Password you would like encrypt with \n:")
    with open(fileToEncrypt, 'rb') as f:
        status = gpg.encrypt(f,
        encrypt=False,
        symmetric='AES256',
        passphrase=passphrase,
        armor=False,
        output=fileToEncrypt + ".gpg")

def ipfsFileFunction(encryptedFile):
    # Tell module where IPFS instance is located
    api = ipfsapi.connect('127.0.0.1', 5001)
    # Add Encrypted file to IPFS
    ipfsLoadedFile = api.add(encryptedFile)
    # Return Hash of new IPFS File
    ipfsHash = (ipfsLoadedFile['Hash'])
    return ipfsHash

def delEncryptedFile(encryptedFile):
    if os.path.isfile(encryptedFile):
        os.remove(encryptedFile)
    else:
        print("Error: %s file not found" % encryptedFile)

def main():
    encryptFileFunction()
    ipfsFileFunction(encryptedFile)
    print ("File encrypted and added to IPFS with this hash " +  ipfsFileFunction(encryptedFile))
    delEncryptedFile(encryptedFile)

main()
\$\endgroup\$
2
\$\begingroup\$

Some things:

  • You'll want to wrap the call to main in if __name__ == '__main__': to be able to import from your package in other packages.
  • For maximum flexibility use argparse to pass the file name rather than making an interactive script.
  • Format strings are generally preferred over string concatenation, so fileToEncrypt + ".gpg" becomes "{}.gpg".format(fileToEncrypt).
  • Run the code through pycodestyle to get a bunch of tips on writing idiomatic Python. For example, Python appropriately uses snake_case for variable, function and method names.
  • Function in a function name is completely redundant. Use a highlighting editor or (ideally) an IDE to trivially distinguish between functions and variables.
  • Magic strings like encryption parameters and the IPFS address should be moved to either a configuration file, a command line parameter (possibly with a default) or a constant at the top of the file.
  • The shebang line should be #!/usr/bin/env python3 for maximum portability - it basically means that the first python3 executable on the PATH will be used.
  • You have a race condition in delEncryptedFile. Rather than checking for a file and then deleting it you should just try to delete it and then let the exception bubble up to the user if it fails. If at some point you want a nicer formatted error message that should be generated in main, not further down, so that, again, other packages can use your package easily.
\$\endgroup\$
  • \$\begingroup\$ Thanks so much! I will be going through your suggestions and making the changes/updates you suggest. \$\endgroup\$ – Troy Wilson Apr 24 '18 at 4:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.