Cloud storage using python Socket library

Question

One year ago I learned some Python 3 in Highschool, but I haven't really programmed anything since then. I decided to start a new project to try and practice a bit before entering the first year of an IT degree. I tried to create my own cloud storage system in a console application using the Socket library, i.e. a program which can send file to a server and retreive them later. I wrote both the client and the server script. It was intended to be only an exercise at first, but I may end up actually using it if I can make it good enough.

Since I wasn't at all familiar with the Socket library, I based my code off of this tutorial, but I now think I've made the code different enough to call it my own.

I've put quite some time into this project and now that I got everything working, I'd really like some feedback. In particular, and in order of importance :

• Did I pick up any bad habbits ?
• Could I make this look cleaner, for exemple by modifing synthax, or adding or fusing fuctions ?
• Is my commenting any good ? Should I comment more or less ? Is it clear enough ?
• Is there any way to optimize the transfer ?

Of course, any criticism is good to take so any remark regarding anything is welcome.

Notes:

• This program is meant to transfer file only and trying to download a directory from the server will cause an error. It is however a non-issue as you cannot select a directory to upload from the the client and the server's storage directory should not contain anything that wasn't sent by the client under normal circumstances.
  • If you decide to manually add files in the server's storage directory, keep in mind that directories are not supported.
  • Modifying the content of the server's storage directory after selecting the option to download files on the client and before you canceled the selection or all downloads are complete may result in an error
• I will be the only person using this app so it isn't useful to make it able to handle multiple requests at the same time.
• Both the computers I use are connected to the same network.
• The app has been tested and works on both Windows and Linux.
  • The client needs to run in an environment capable of showing a GUI as I use tkinter's filedialog to select the files to send to the server. This not the case for the server.
• I use python 3.8.2

---

Here is server script:

import socket
import tqdm
import os
import pickle
import sys

##Setup connection with client

#Set storage directory
workDir = "Insert path to server's storage directory"
os.chdir(workDir)

#Set global variable
SERVER_HOST = "0.0.0.0"
SERVER_PORT = 5001
BUFFER_SIZE = 1024
SEPARATOR = "<SEPARATOR>"

#Start listening for connection attempt and perform the action chosen by the client
def start_listening():
    #Accept the connexion of the client
    s = socket.socket()
    s.bind((SERVER_HOST, SERVER_PORT))
    s.listen(1)
    print(f"-------------------------\nListening to {SERVER_HOST}:{SERVER_PORT}")
    client_socket, CLIENT_HOST = s.accept()
    print(f"{CLIENT_HOST} is connected.")

    #Perform the action chosen by the client
    actionID = client_socket.recv(BUFFER_SIZE).decode()
    client_socket.send('0'.encode())
    if actionID == '1':
        receive_file_choice(client_socket)
    if actionID == '2':
        send_file_choice(client_socket)
    if actionID == '3':
        client_socket.close()
        s.close()
        sys.exit()

    #Restart the script to perform another action
    client_socket.send('0'.encode())
    client_socket.close()
    s.close()
    start_listening()

##Download

#Receive the number, names and sizes of the files to download
def receive_file_choice(client_socket):
    fileNumber = int(client_socket.recv(BUFFER_SIZE).decode())
    client_socket.send('0'.encode())

    #Control the receive_file() function
    for i in range(fileNumber):
        received = client_socket.recv(BUFFER_SIZE).decode()
        fileName, fileSize = received.split(SEPARATOR)
        fileName = os.path.basename(fileName)
        fileSize = int(fileSize)
        client_socket.send('0'.encode())

        print(f"\nDownloading {fileName} ({fileSize} bytes)")
        receive_file(client_socket, fileName, fileSize)

#Download the file sent by the client and store them in workDir
def receive_file(client_socket, fileName, fileSize):
    client_socket.send('0'.encode())

    with open(fileName, "wb") as f:

        downloadSize = 0
        timeOutFlag = 0
        #Setting a time out here ensure the the script can continue even if a packet was lost
        client_socket.settimeout(10)
        #Receive packets of size BUFFER_SIZE bytes
        for i in tqdm.tqdm(range(fileSize//BUFFER_SIZE)):
            try:
                downloadSize += f.write(client_socket.recv(BUFFER_SIZE))
            except (socket.timeout):
                print('Error : Download timed out')
                timeOutFlag = 1
                break
            #Tell the client it can send the next packet
            client_socket.send('0'.encode())
        #Receive remainig bytes
        if timeOutFlag == 0:
            try:
                downloadSize += f.write(client_socket.recv(fileSize%BUFFER_SIZE))
            except socket.timeout:
                pass
        client_socket.settimeout(None)

        #Warn the user if data has been lost during the transfer
        if downloadSize == fileSize:
            print("Done")
        else:
            print(f"Warning: '{fileName}' does not match expected size\n(Received {downloadSize} bytes instead of {fileSize} - {fileSize - downloadSize} bytes difference)")

    #Acknowledge that the download as ended. Send a '1' instead of '0' so that the client don't confuse it with the 'packet received' acknowledgement
    client_socket.send('1'.encode())

##Upload

#Send a list of available files and retreive client's choice
def send_file_choice(client_socket):
    print('Sending a list of available files to client')
    tempFileList = os.listdir(workDir)

    #If files are available to download for the client
    if len(tempFileList) >= 1:
        fileList = [(i, tempFileList[i], os.path.getsize(tempFileList[i])) for i in range(len(tempFileList))]
        sentFileList = pickle.dumps(fileList)

        client_socket.send(str(len(sentFileList)).encode())
        client_socket.recv(1)
        client_socket.send(sentFileList)
        print('Waiting for client response')


        choiceList = client_socket.recv(BUFFER_SIZE).decode()

        #Return if the client has chosen to cancel the download
        if choiceList.lower() == "-c":
            print('Request canceled')
            return

        choiceList = choiceList.split(',')
        client_socket.send('0'.encode())

        for i in range(len(choiceList)):
            choiceList[i] = int(choiceList[i])

        fileNumber = len(choiceList)
        client_socket.send(str(fileNumber).encode())
        client_socket.recv(1)

        control_upload(client_socket, fileList, choiceList)

    #Return if no file was found
    else:
        client_socket.send(pickle.dumps('0'))
        print('No file was found')
        return

#Operates the send_file() function
def control_upload(client_socket, fileList, choiceList):
    for i in choiceList:
        fileName, fileSize = fileList[i][1], fileList[i][2]
        client_socket.send(f"{fileName}{SEPARATOR}{fileSize}".encode())
        client_socket.recv(1)

        print(f"\nUploading {fileName} ({fileSize} bytes)")
        send_file(client_socket, fileName, fileSize)

#Send requested file to the client
def send_file(client_socket, fileName, fileSize):
    client_socket.recv(1)

    with open(fileName, "rb") as f :

        #Upload the file divided in packet of size BUFFER_SIZE bytes
        for i in tqdm.tqdm(range(fileSize//BUFFER_SIZE)):
            bytes_read = f.read(BUFFER_SIZE)
            client_socket.sendall(bytes_read)
            client_socket.recv(1)
        #Send the remaining bytes
        bytes_read = f.read(fileSize%BUFFER_SIZE)
        client_socket.sendall(bytes_read)

        #Wait for the server to acknowledge it has ended downloading the file
        while True:
            if client_socket.recv(1).decode() == '1':
                break
        print("Done")

if __name__ == '__main__':
    start_listening()

Here is client script:

import tkinter as tk
from tkinter import filedialog
import os
import tqdm
import socket
import pickle
from time import sleep

##Set up connection with server

#Set storage directory
workDir = "Insert path to your download directory here"
os.chdir(workDir)

#Set global variables
SERVER_HOST = "Insert server's IP adress here"
SERVER_PORT = 5001
BUFFER_SIZE = 1024
SEPARATOR = "<SEPARATOR>"

#Start the connection with the server, choose the action to perfmorm
def start_connection():
    #Connect to the server
    s = socket.socket()
    print(f"-------------------------\nConnecting to {SERVER_HOST}:{SERVER_PORT}")
    #Setting a timeout here prevent the client from trying to connect forever
    s.settimeout(30)
    try:
        s.connect((SERVER_HOST, SERVER_PORT))
    except socket.timeout:
        print("Error : Connection timed out")
        return
    #Maintaining the timeout past this point could cause an error if the user took a long time to decide what he want to do. timeout is therefore set to None.
    s.settimeout(None)
    print("Connected")

    #Ask for the user to choose the action to perform until he enter a valid action
    while True:
        actionID = input("\nChoose action to perform:\n1. Upload\n2. Download\n3. Close server\n4. Exit\n-> ")
        if actionID in ['1','2','3','4']:
            break
        else:
            print('Invalid action')

    #Perform the requested action
    if actionID == '1':
        get_file_list(s)
    if actionID == '2':
        receive_file_choice(s)
    if actionID == '3':
        confirm = input("Do you want to procede? You won't be able to restart the server without physical access. (y/n)\n-> ")
        if confirm == 'y' or confirm == 'Y':
            s.send('3'.encode())
            print("Server closed")
            return
        else:
            print("Server was not closed")
            return
    if actionID == '4':
        print('Exited')
        return

    #Restart the script to perform another action
    s.recv(1)
    sleep(0.2)
    s.close()
    start_connection()

##Upload

#Make the user choose the files to upload
def get_file_list(s):
    s.send('1'.encode())
    s.recv(1)

    #Open the file selection window
    selectionWindow = tk.Tk()
    selectionWindow.withdraw()

    fileNames = filedialog.askopenfilenames(parent=selectionWindow, initialdir=os.getcwd(), title='Please select file(s)')
    fileNumber = len(fileNames)
    fileSizes = tuple([os.path.getsize(fileNames[i]) for i in range(fileNumber)])

    selectionWindow.destroy()

    #Control the send_file() function
    s.send(str(fileNumber).encode())
    s.recv(1)
    for i in range(fileNumber):
        fileName, fileSize = fileNames[i], fileSizes[i]
        s.send(f"{fileName}{SEPARATOR}{fileSize}".encode())
        s.recv(1)

        print(f"\nUploading {fileName} ({fileSize} bytes)")
        send_file(s, fileName, fileSize)

#Send a file to the server
def send_file(s, fileName, fileSize):
    s.recv(1)

    with open(fileName, "rb") as f :

        #Upload the file divided in packet of size BUFFER_SIZE bytes
        for i in tqdm.tqdm(range(fileSize//BUFFER_SIZE)):
            bytes_read = f.read(BUFFER_SIZE)
            s.sendall(bytes_read)
            s.recv(1)
        #Send the remaining bytes
        bytes_read = f.read(fileSize%BUFFER_SIZE)
        s.sendall(bytes_read)

        #Wait for the server to acknowledge it has ended downloading the file
        while True:
            if s.recv(1).decode() == '1':
                break
        print("Done")

##Download

#Ask for the user to choose the files to download
def receive_file_choice(s):
    s.send('2'.encode())
    s.recv(1)

    #Receive the list of all available files on the server
    print('Waiting for a list of available files')
    fileListSize = int(s.recv(BUFFER_SIZE).decode())
    s.send('0'.encode())
    sentFileList = s.recv(fileListSize)
    sentFileList = pickle.loads(sentFileList)

    #Return if no file is available
    if sentFileList == '0':
        print('No file is available')
        return

    else:
        print('\nID ; Name ; Size\n----------------')

        possibleChoiceList = []
        for i in range(len(sentFileList)):
            print(f"{sentFileList[i][0]} ; {sentFileList[i][1]} ; {sentFileList[i][2]}")
            possibleChoiceList.append(str(sentFileList[i][0]))

        #Ask for the user to chose file until a valid entry is given
        while True:
            choiceListStr = input('\nEnter the IDs of the files to request separated with commas\nEnter "-c" to cancel\n-> ')

            #Return if the user wants to cancel the download
            if choiceListStr.lower() == "-c":
                print('Request canceled')
                s.send(choiceListStr.encode())
                return

            choiceList = choiceListStr.split(',')
            #Check if the entry contains something that is not associated with a file (e.g., a negative number, a random letter...)
            if not set(choiceList).issubset(set(possibleChoiceList)):
                print('You cannot choose a file that is not listed')
            #Check if a file is requested twice
            elif len(choiceList) != len(set(choiceList)):
                print('You cannot choose a file more than once')
            #Break the loop if the entry is valid
            else:
                break

        s.send(choiceListStr.encode())
        s.recv(1)
        control_download(s)

#Operate the receive_file() function
def control_download(s):
    print('Waiting for the server to send files')
    fileNumber = s.recv(BUFFER_SIZE).decode()
    fileNumber = int(fileNumber)
    s.send('0'.encode())
    print(f"Downloading {fileNumber} files")

    for i in range(fileNumber):
        receive = s.recv(BUFFER_SIZE).decode()

        fileName, fileSize = receive.split(SEPARATOR)
        fileName = os.path.basename(fileName)
        fileSize = int(fileSize)

        s.send('0'.encode())

        print(f"\nDownloading {fileName} ({fileSize} bytes)")
        receive_file(s, fileName, fileSize)

#Receive a file from the server and store it in workDir
def receive_file(s, fileName, fileSize):
    s.send('0'.encode())

    with open(fileName, "wb") as f:

        downloadSize = 0
        timeOutFlag = 0
        #Setting a timeout here ensure the the script can continue even if a packet was lost
        s.settimeout(10)
        #Receive packets of size BUFFER_SIZE bytes
        for i in tqdm.tqdm(range(fileSize//BUFFER_SIZE)):
            try:
                downloadSize += f.write(s.recv(BUFFER_SIZE))
            except socket.timeout :
                print("\nError : Download timed out")
                timeOutFlag = 1
                break
            #Tell the server it can send the next packet
            s.send('0'.encode())
        #Receive the remainig bytes
        if timeOutFlag == 0:
            try:
                downloadSize += f.write(s.recv(fileSize%BUFFER_SIZE))
            except socket.timeout:
                pass
        s.settimeout(None)

        #Warn the user if data has been lost during the transfer
        if downloadSize == fileSize:
            print("Done")
        else:
            print(f"Warning: '{fileName}' does not match expected size\n(Received {downloadSize} bytes instead of {fileSize} - {fileSize - downloadSize} bytes difference)")

    #Acknowledge that the download as ended. Send a '1' instead of '0' so that the server don't confuse it with the 'packet received' acknowledgement
    s.send('1'.encode())

if __name__ == "__main__":
    start_connection()

---

When choosing the files to download on the client, you should see something like this

ID ; Name ; Size
----------------
0 ; 3tseT.txt ; 7
1 ; Script1.py ; 5102
2 ; Script2.py ; 4835
3 ; NicePDF.pdf ; 193721
4 ; Test1.txt ; 22
5 ; Test2.txt ; 16
6 ; Test3bis.txt ; 1

Enter the IDs of the files to request separated with commas
Enter "-c" to cancel
->

If you want to select the files 0, 1, 4 and 5, enter 0,1,4,5 in the console.

---

I tried transfering files using

with open(filename, "rb") as f:
    while True:
        bytes_read = f.read(BUFFER_SIZE)
        if not bytes_read:
            break
        s.sendall(bytes_read)

and

with open(filename, "wb") as f:
    while True:
        bytes_read = client_socket.recv(BUFFER_SIZE)
        if not bytes_read:    
            break
        f.write(bytes_read)

which would have been simpler, but it ended up fusing files together when downloading multiple at once, so I opted for what you can see above.

Also, even though the send('0'.encode()) and recv(1) significantly slow the transfer down, they are necessary to ensure that the client and the server work synchonously, and getting rid of them causes data loss. Increasing the buffer size can help speed up the process but setting it too high causes data loss as well.

---

Finally, as english isn't my native language, I apology for any mistake I could have made. Feel free to correct me or ask for clarification.

Thanks for your time and have a nice day.

---

Edit: Created a github repository to update the code as I receive suggestions.

Answer 1

• Let's start with the easy stuff. Naming. CloudPy-Client.py is not, by any means, a Pythonic name. cloudpy_client.py is what one would expect. Same for server.
• Client and Server both belong to CloudPy. Why not create a package and avoid the name clutter?

/cloudpy
    - __init__.py
    - client.py
    - server.py

• If a variable is a constant, it should by all uppercase. work_dir -> WORK_DIR.
• Statically defining the working directory is not nice. If I want to run it, I need to change it. Check out this.
• Again, this does not follow Python's naming convention. action_ID -> action_id
• Why code repetition in both files for constants? Move it to a config.py or constants.py and share it betweent he two files.

SERVER_HOST = "192.168.1.10"
SERVER_PORT = 5001
BUFFER_SIZE = 1024
SEPARATOR = "<SEPARATOR>"

• PEP8 specifies functions in Python should have two blank lines above for style. E.g., CloudPy-Server.py line 20. Are you not using an IDE like Pycharm? It warns you of all of these style "errors".
• Avoid inline magic values.

    if action_ID == "1":
        receive_file_choice(client_socket)
    if action_ID == "2":
        send_file_choice(client_socket)
    if action_ID == "3":
        client_socket.close()
        s.close()
        sys.exit()

->

    if action_ID == ACTION_RECEIVE:
        receive_file_choice(client_socket)
    if action_ID == ACTION_SEND:
        send_file_choice(client_socket)
    if action_ID == ACTION_CLOSE:
        client_socket.close()
        s.close()
        sys.exit()

• Python has a maximum recursion depth. Recursion is expensive. Why use recursion when you can use a while loop?

def start_listening():
    ...
    start_listening()

def main():
    while True:
        start_listening()

• Again, style, your IDE should be warning you about this. No blank line at the end of file.
• Again, style, ##Setup connection with client -> # setup connection with client.
• This is important. Mixing UI with logic is a terrible idea and signals you are picking up bad practices (of course, might not be true, this is my guess). Separate your UI and program flow, from the logic. client_ui.py could be in charge of askopenfile_names and then, once that is done, reach out to client.py with the files the user has chosen and do the logic there.
• Remember SRP. One function ideally does one thing. Start connection should start a connection, not start a connection, ask an action id, ......... Divide them. Use auxiliary functions, e.g., ask_action, get_connection, etc.
• What does this sleep do? Why is it there? sleep(0.2) Add a comment for non-trivial stuff!
• If a variable is unused, convention says we should use _ as its name. for i in tqdm.tqdm(range(file_size // BUFFER_SIZE)): -> for _ in tqdm.tqdm(range(file_size // BUFFER_SIZE)):.

There surely is a lot more, but I feel like this is enough for now... If you fix all of this and want more, ping me on the comments!

---

On to more!

• File structure, again.
  1. How does the name modules help you? Trivially, a package will contain modules. Let's choose more informative names. E.g., backend or hosts.
  2. What does ui.py have in common with client.py or server.py? If we have 2 "UIs" in cloudpy.py, wouldn't it make more sense to have server_cli.py and client_cli.py?
  3. Why is the logic of def client_side(): and def server_side(): with the UI? What is the difference? I buy that def chose_side(): is a common entry point, so that makes sense as a separate file, but the other two would make much more sense with their respective UIs. If you want to have common utils between server_cli.py and client_cli.py, you can add it /controllers/common.py or similar.

/modules
    - __init__.py
    - client.py
    - server.py
    - ui.py
cloudpy.py
config.py

/hosts
    - __init__.py
    - client.py
    - server.py
/controllers
    - __init__.py
    - client_cli.py
    - server_cli.py
    - common_cli.py
cloudpy.py
config.py

or if you plan in building a non-cli UI:

/hosts
    - __init__.py
    - client.py
    - server.py
/controllers
    - __init__.py
    /cli
        - __init__.py
        - client_cli.py
        - server_cli.py
        - common_cli.py
cloudpy.py
config.py

• Avoid wildcard imports. Read about it here and/or here.
• print works well for prototyping or putting something together quickly. Printing makes sense in the cli UI (e.g. ui.py, or the new modules in client_cli.py and similar). There we are outputting text as a means to output text. If instead what we are doing is logging, like in client.py or server.py, then use a log and not a print! Read more about it here and here.
• If you end up using them as sets, why declare them as lists? choice_list and possible_choice_list. Just have sets from the beginning.
• receive_file_choice() is poorly architected. receive_file_choice is supposed to be part of the logic functions (it is in client.py!). Thus, it should not be asking for input to the user in ui.get_download_file_list(sent_file_list). It must be the other way around, the UI (i.e. client_cli.py) must call receive_file_choice(server_socket, choice_list_str, choice_list) with everything it needs. Same in any other similar place.
• Same later down the function: the UI should be doing all of this validation and printing, and then calling some cancel() or whatever.

if choice_list_str.lower() == CANCEL_DOWNLOAD:
            print("Request canceled")
            server_socket.send(choice_list_str.encode())
            return CANCEL

• Add type hinting! Good for checking for finding stupid mistakes with the IDE, code autocompletion and suggestions, and having a self-documenting code. Read about it here and here.
• What is this function doing? Does not follow SRP and is unclear. Is it choosing an action or acting upon it? For UPLOAD and DOWNLOAD it seems to be just returning the id. While for CLOSE_SERVER it does not return anything and instead performs an action!

# Perform the action chosen by the client
def chose_action(client_socket):
    action_id = client_socket.recv(BUFFER_SIZE).decode()
    client_socket.send("0".encode())

    if action_id == UPLOAD:
        return UPLOAD

    if action_id == DOWNLOAD:
        return DOWNLOAD

    if action_id == CLOSE_SERVER:
        print("Closing server")
        sys.exit()

• Simplify your code making use of exceptions. I.e., just raise a ValueError. Or implement your custom error (e.g. class NoFileError(ValueError)) and raise that. Then you can catch it later. We don't need to be having some return codes like CANCEL, we can just raise an exception if something did not work as we expected and catch it wherever we can print feedback on that exception to the cli or any other UI.

else:
        sent_file_list = pickle.dumps(NO_FILE)
        client_socket.send(str(len(sent_file_list)).encode())
        client_socket.recv(1)
        client_socket.send(sent_file_list)
        print("No file was found")
        return CANCEL

• You mentioned

I will be the only person using this app so it isn't useful to make it able to handle multiple requests at the same time.

That is IMO not true. Handling multiple requests at the same time makes sense even with 1 user. If a user wants multiple files, it is """equivalent""" to multiple users wanting one file. Using threading (or possibly asyncio, but would have to check in the case of asyncio if it actually can help) you would be able to speed it up for 1+ files, since the bottleneck is the network and the file reading... Let me know if you are interested in exploring this in the future. This might interest you anyway.

There are still more things for sure, but I feel this is, once again, more than enough for one CR!

---

Let's see more, now that it is cleaner!

• config.py is supposed to be only a file for some constants. Opening and making files or directories is not good. Why? Every time we import config we will run the whole script to check if the path exists, create dirs or files, etc. How can we fix it? Is log and storage for the client or server? Put it in the UI, which is the client that runs. There is where creating dirs/files should be. Plus, then you can control and make sure it only happens when the user starts the client/server, and not every time we import config.

• If something in the config file is different for client and server. Maybe WORK_DIR or LOG_FILE it shouldn't be in the same config. Either it is not being used or it is missleading. You can have a config for the client and server specific settings, and the common one.

• You can add a if __name__ == '__main__' in both controllers/cli/client_cli.py and controllers/cli/server_cli.py, so that you can directly run each of the components without choosing side. Not strictly necessary, but makes sense and is useful. E.g., for the client you would have if __name__ == '__main__': client_side().

• Naming for def server_side(): and def client_side(): is not the most self-explanatory. Maybe def run_client(): and def run_server():, or just def main(): to clearly indicate that it is the entry point to the client/server app.

• Now let's go into a more advance topic and explanation about this design and responsibilities.

TLDR: using sys.exit() in the logic side (in the hosts) is definitely not recommended.

Remember, we have split in one side the UI (cli in this case), and in the other all the logic. This way we can reuse the logic, or change the UI, and keep the two independent. Who is in charge of running the client/server and the application flow? The controller! Not the logic function. start_connection() should only start a connection, not start a connection and exit the program if it doesn't work.

Imagine you have a boss, and you find out there is a security bug somewhere. In this metaphor, you would raise your concern to your boss, who in his place would raise it to the ultimate boss, who might decide to e.g. stop the servers until is resolved. You would not just go to the ultimate boss, or pull the plug on the server. That's not your responsibility!

Same happens in code. In this case we have something like this:

+------------------------+------------+---+-------------+
|                        |    user    |   | file system |
+------------------------+------------+---+-------------+
|                        |     ↑ ↓    |   |     ↑ ↓     |
+------------------------+------------+---+-------------+
| UI layer (controllers) | CLIENT_CLI |   | SERVER_CLI  |
+------------------------+------------+---+-------------+
|                        |     ↑ ↓    |   |     ↑ ↓     |
+------------------------+------------+---+-------------+
| logic layer (hosts)    |   client   | ↔ |    server   |
+------------------------+------------+---+-------------+

What you are doing by calling sys.exit() in hosts/client.py (logic layer) is effectively going beyond the responsibilities of your layer. Something like this:

+------------------------+------------+---+-------------+
|                        |    user    |   | file system |
+------------------------+------------+---+-------------+
|                        |↗    ↑ ↓    |   |     ↑ ↓     |
+------------------------+------------+---+-------------+
| UI layer (controllers) |↑CLIENT_CLI |   | SERVER_CLI  |
+------------------------+------------+---+-------------+
|                        |↑    ↑ ↓    |   |     ↑ ↓     |
+------------------------+------------+---+-------------+
| logic layer (hosts)    |↖  client   | ↔ |    server   |
+------------------------+------------+---+-------------+

HOW TO SOLVE IT? If you have an error in hosts/client.py, raise an exception to notify to the UI layer that something went wrong. And the upper layer can catch it, and then sys.exit(), since it is their responsibility.

• Same happens in close_server(). close_server() should notify to the server he should close, not close the server AND exit the client.

• Similarly, why would we have exit_client() in hosts/client.py, exiting the app is responsibility of the CLI GUI layer.

• In request_action() -> str, again, you violate SRP. request_action should request an action. And since it only requests an action, how can we know if it was executed successfully? Printing server closed or exited there makes no sense, since we don't know whether it actually performed the action correctly or not. So this should be printed somewhere, probably in client_side.

• hosts/client.py def chose_action(socket, action_id) makes no sense (the name). You are not choosing and action, but performing the action which was already chosen. Plus there is a typo in chose.

• The log file is ok, but usually we add the date to the log file, so that you have one log file per day. Or something like this.

• I think you can now implement custom errors to fix the sys.exit issue in the logic layer. Read here and here.

• And also threading to speed things up.

  1. First read a bit about the options for concurrency in Python: here, here. And remember your problem is an I/O bound problem, since the bottleneck is sending the files, not doing anything computationally expensive with them.
  2. Now, how to do this?

+------------+--------------------+--------------------------------------------------------------------+
|            |        PORT        | DESCRIPTION                                                        |
+------------+--------------------+--------------------------------------------------------------------+
|   control  | SERVER_PORT (5001) | used for communicating between client <-> server                   |
|            |                    |                                                                    |
|            |                    | This requests jobs and does all the logistics                      |
|            |                    | (closing server, cancelling downloads, requesting downloads...).   |
|            |                    |                                                                    |
|            |                    | Once the logistics are done and everything is ready for the actual |
|            |                    | file transfer, it send assigns an *operation* port where           |
|            |                    | the actual file transfer gets done.                                |
+------------+--------------------+--------------------------------------------------------------------+
| operations |     5002 - 5020    | Used for a concrete expensive operation, like sending one file.    |
+------------+--------------------+--------------------------------------------------------------------+

E.g., client requests to download a folder called john.

\john
    - test1.txt
    - test2.txt
    - test3.txt

The process would go like follows: - client requests the folder to the server in the control port, i.e. 5001 - server allocates some new parallel threads to send the file in some specific ports, e.g. 5002 - 5004 - client allocates some new parallel threads for each job, and requests each file in parallel to each of the ports the server allocated for the jobs - once everything is done, e.g., the user chooses to close the server, so the client sends an exit message to the server through the control port (5001)

+--------+--------------------------------+---+---------------------------------------------------------+-------------+
|        | CLIENT                         |   | SERVER                                                  | SERVER_PORT |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 0      | I want to download dir '\john' | > |                                                         |     5001    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 1      |                                | < | Ok! It has 3 files. Request each one individually here: |     5001    |
|        |                                |   | {                                                       |             |
|        |                                |   |     'test1.txt': 5002,                                  |             |
|        |                                |   |     'test2.txt': 5003,                                  |             |
|        |                                |   |     'test3.txt': 5004                                   |             |
|        |                                |   | }                                                       |             |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 2      | ACK!                           | > |                                                         |     5001    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 3      | Can we start downloading?      | > |                                                         |     5002    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 4      | Can we start downloading?      | > |                                                         |     5003    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 5      | Can we start downloading?      | > |                                                         |     5004    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 6 - 8  |                                | < | Sure!                                                   |     5002    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 6 - 8  |                                | < | Sure!                                                   |     5003    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 6 - 8  |                                | < | Sure!                                                   |     5004    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 9 - 10 |                                | < | {test1.txt}                                             |     5002    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 9 - 10 |                                | < | {test2.txt}                                             |     5003    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 9 - 10 |                                | < | {test3.txt}                                             |     5004    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 11     | Close server!                  | > |                                                         |     5001    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+
| 12     |                                | < | Ok, bye!                                                |     5001    |
+--------+--------------------------------+---+---------------------------------------------------------+-------------+

I hope this makes some sense and helps you get started! If you get stuck, just ping me :)

This example using a ThreadPoolExecutor will help you. This one and this one are also nice!

• Finally, what about adding encryption so that the file transfer does not travel unencrypted directly on the wire? Check this and this!