Облачное хранилище с использованием библиотеки сокетов Python

Год назад я выучил Python 3 в старшей школе, но с тех пор я практически ничего не программировал. Я решил начать новый проект, чтобы немного попрактиковаться перед поступлением на первый год обучения в сфере ИТ. Я попытался создать свою собственную облачную систему хранения в консольном приложении, используя библиотеку Socket, то есть программу, которая может отправлять файлы на сервер и получать их позже. Я написал и клиентский, и серверный скрипты. Сначала это было всего лишь упражнение, но я могу в конечном итоге использовать его, если смогу сделать его достаточно хорошим.

Поскольку я совершенно не был знаком с библиотекой Socket, я основывал свой код на этот учебник, но теперь я думаю, что сделал код достаточно другим, чтобы называть его своим.

Я потратил некоторое время на этот проект, и теперь, когда у меня все работает, мне действительно нужны отзывы. В частности, и в порядке важности:

• Приобрел ли я какие-нибудь дурные привычки?
• Могу ли я сделать это более чистым, например, изменив синтаксис или добавив или объединив функции?
• Мои комментарии хороши? Стоит ли комментировать более или менее? Достаточно ли ясно?
• Есть ли способ оптимизировать перевод?

Конечно, можно принимать любую критику, поэтому любые замечания по поводу чего-либо приветствуются.

Примечания:

• Эта программа предназначена только для передачи файлов, и попытка загрузить каталог с сервера вызовет ошибку. Однако это не проблема, поскольку вы не можете выбрать каталог для загрузки с клиента, а каталог хранилища сервера не должен содержать ничего, что не было отправлено клиентом при обычных обстоятельствах.
  • Если вы решили вручную добавить файлы в каталог хранилища сервера, имейте в виду, что каталоги не поддерживаются.
  • Изменение содержимого каталога хранилища сервера после выбора варианта загрузки файлов на клиенте и до того, как вы отменили выбор или все загрузки были завершены, может привести к ошибке.
• Я буду единственным, кто использует это приложение, поэтому бесполезно заставлять его обрабатывать несколько запросов одновременно.
• Оба компьютера, которые я использую, подключены к одной сети.
• Приложение протестировано и работает как в Windows, так и в Linux.
  • Клиент должен работать в среде, способной отображать графический интерфейс, поскольку я использую диалоговое окно tkinter для выбора файлов для отправки на сервер. Это не относится к серверу.
• Я использую python 3.8.2

Вот сценарий сервера:

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 sizen(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()

Вот клиентский скрипт:

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. Uploadn2. Downloadn3. Close servern4. Exitn-> ")
        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 ; Sizen----------------')

        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 commasnEnter "-c" to canceln-> ')

            #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 sizen(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()

При выборе файлов для загрузки на клиенте вы должны увидеть что-то вроде этого

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
->

Если вы хотите выбрать файлы 0, 1, 4 и 5, введите 0,1,4,5 в консоли.

Я пробовал передавать файлы с помощью

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

а также

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

что было бы проще, но это привело к слиянию файлов при одновременной загрузке нескольких, поэтому я выбрал то, что вы видите выше.

Кроме того, хотя send('0'.encode()) а также recv(1) значительно замедляют передачу, они необходимы для того, чтобы клиент и сервер работали синхронно, и избавление от них приводит к потере данных. Увеличение размера буфера может помочь ускорить процесс, но установка слишком большого размера также приводит к потере данных.

Наконец, поскольку английский не является моим родным языком, я прошу прощения за любую ошибку, которую мог совершить. Не стесняйтесь поправлять меня или просить разъяснений.

Спасибо за уделенное время и хорошего дня.

Изменить: создал репозиторий github обновлять код по мере поступления предложений.

• Начнем с простого. Именование. CloudPy-Client.py ни в коем случае не является питоническим именем. cloudpy_client.py это то, что можно было ожидать. То же самое для сервера.
• Клиент и сервер принадлежат CloudPy. Почему бы не создать пакет и избежать беспорядка в именах?

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

• Если переменная является константой, она должна быть в верхнем регистре. work_dir -> WORK_DIR.
• Статическое определение рабочего каталога нехорошо. Если я хочу запустить его, мне нужно его изменить. Проверить это.
• Опять же, это не соответствует соглашению об именах Python. action_ID -> action_id
• Зачем повторение кода в обоих файлах для констант? Переместите его в config.py или constants.py и поделитесь им между двумя файлами.

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

• PEP8 указывает, что функции в Python должны иметь две пустые строки выше для стиля. Например, строка 20. CloudPy-Server.py. Вы не используете IDE, такую ​​как Pycharm? Он предупреждает вас обо всех этих «ошибках» стиля.
• Избегайте встроенных магических значений.

    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 имеет максимальную глубину рекурсии. Рекурсия стоит дорого. Зачем использовать рекурсию, если можно использовать цикл while?

def start_listening():
    ...
    start_listening()

def main():
    while True:
        start_listening()

• Опять же, стиль, ваша IDE должна предупреждать вас об этом. В конце файла нет пустой строки.
• Опять же стиль, ##Setup connection with client -> # setup connection with client.
• Это важно. Смешивание пользовательского интерфейса с логикой — ужасная идея и сигнализирует о том, что вы принимаете плохие методы (конечно, это может быть неправдой, это мое предположение). Разделите свой пользовательский интерфейс и поток программы, чем логику. client_ui.py мог бы отвечать за askopenfile_names а затем, как только это будет сделано, обратитесь к client_ui.py с файлами, выбранными пользователем, и выполните там логику.
• Помните SRP. Одна функция в идеале выполняет одну задачу. Начать соединение должно запускать соединение, а не начинать соединение, запрашивать идентификатор действия, ……… разделить их. Используйте вспомогательные функции, например, ask_action, get_connection, так далее.
• Что делает этот сон? Почему это там? sleep(0.2) Добавляйте комментарии по нетривиальным вещам!
• Если переменная не используется, по соглашению мы должны использовать _ как его имя. for i in tqdm.tqdm(range(file_size // BUFFER_SIZE)): -> for _ in tqdm.tqdm(range(file_size // BUFFER_SIZE)):.

Конечно, есть намного больше, но я чувствую, что на данный момент этого достаточно … Если вы исправите все это и захотите большего, напишите мне в комментариях!