0
\$\begingroup\$

I have a program that includes sending data to our backend servers upon completion.

Due to the occasional connection issue at either side I have coded into the main script a JSON dump as a backup in case we are unable to connect to the BE server.

The JSON file is simply a Key/Value mirror of the SQL database fields in the table in question.

I will run this program in the background which will periodically check the folder. It will open the JSON and insert / update any files that are in there and remove them if completed.

from json import load
from time import sleep
from os.path import join
from os import remove
from glob import glob

from sql_tunnel_control import (
    SqlTunnelConnect,
    SSH_KEY,
    sql_ip,
    sql_hostname,
    sql_username,
    sql_password,
    sql_main_database,
    sql_port,
    ssh_host,
    ssh_port,
    ssh_user,
    ssh_password,
    table
)

production = SqlTunnelConnect(
    SSH_KEY,
    None,
    sql_hostname,
    sql_username,
    sql_password,
    table,
    sql_port,
    ssh_host,
    ssh_port,
    ssh_user,
    None,
)


def load_be_data(f):
    """ Save data to json file and allow another program to iterate over that folder until all data is safely in the backend server"""
    try:
        with open(join(f), "r") as j:
            return load(j)
    except Exception as e:
        print(e)
        # pass


def get_files():
    f = glob("backend_data/*.json")
    return f

def send_to_be(f):
    """ takes in a file and attempts to send  to the server.. Returns False if it fails.. Deletes the file if it succeeds """
    print(f)

    if f["query_type"] == "insert":
        create_new_component = """INSERT INTO components .... """
        try:
            production.run_query(create_new_component)
            return True
        except:
            return False
    elif f["query_type"] == "update":
        update_component = """UPDATE components SET ..."""
        try:
            production.run_query(update_component)
            return True
        except:
            return False
    else:
        return False  # problem with query type

def persist_dump():
    to_send = get_files()
    while len(to_send) > 0:
        for f in to_send:
            r = send_to_be(load_be_data(f))
            if r:
                remove(f)
                sleep(1)
            else:
                sleep(300)  # 5 minutes rest.
    # all files completed.. sleep for a bit.. then see whats what//
    sleep(1800)
    persist_dump()

persist_dump()
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

load_be_data

  1. Parameter name is not very descriptive. I'd say that 'file_path' would be better.
  2. Assuming that there is some end user, it would be better to add some descriptive messages for different kinds of exceptions instead of printing the message. Also, it is a good practice to note where the exception was caught. For example, 'Exception with message XYZ was raised in load_the_data'.

get_files

I don't think there is any reason to create a local variable. Just do :

def get_files():
    return glob("backend_data/*.json")

send_to_be

  1. Again, parameter name is not descriptive.
  2. Is there any reason why do you want to return boolean value instead of raising the error? The 'most pythonic' way is to ask for forgiveness than permission EAFP. In my opinion, what should happen here is that the function should raise an exception when something bad happens or continue when everything is okay. This would help you because you wouldn't need to handle errors separately.

Try this in persist_dump:

try:
    send_to_be(load_be_data(f))
    sleep(300)
except (some exception) as ex:
    print("Exception thrown in ...")
    remove(f)
    sleep(1)
  1. Both "insert" and "update" should be a global variables (so you can possibly reuse them somewhere else).
  2. If create_new_component and update_component are not calculated inside the method, it would be good to make them global variables.

persist_dump

I don't think that it is a good idea to call persist_dump recursively. Assuming that you script will run for a while you are using more memory than you need to. Additionally, at some point in the future you might get RuntimeError: maximum recursion depth exceeded.

\$\endgroup\$
1
  • \$\begingroup\$ Not calling persist_dump recursively if what i was looking for..so thanks.. Also some other good points raised which I have implemented :) \$\endgroup\$
    – johnashu
    Sep 17, 2018 at 13:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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