Téléchargeur en Python

Question

J'ai écrit ce code en python et cela fonctionne bien pour moi mais je sais que le code n'est pas optimisé et que beaucoup de refactoring doit être fait. J'ai donc besoin d'un examen sur la façon dont ce code peut être amélioré. J'ai commencé à écrire ceci quand la bibliothèque WGET ne fonctionnait pas pour moi et je voulais un script léger pour mes autres projets. Je pense également à remplacer la bibliothèque de requêtes par aiohttp. Étant un débutant dans ce domaine, j'attends vos critiques avec impatience.

Je vous remercie.

Exigences : - tqdm , requêtes

Implémentation: - downloader.py

import requests
import os
from uuid import uuid4
from urllib.parse import urlparse, unquote
import re
from datetime import datetime
from requests.exceptions import HTTPError, ReadTimeout,InvalidSchema
from requests.adapters import HTTPAdapter
from urllib3.util.retry import Retry
from tqdm import tqdm


class Rget:
  def __init__(self, url, dest=os.getcwd(), filename=None, progress_bar=True, headers=None):
    self.url = url
    self.dest = self.check_if_dir_exist(dest)
    self.filename = filename
    self.progress_bar = progress_bar
    # self.headers = self.fetch_headers(headers)

  def check_if_dir_exist(self, dest):
    """
    Function to check whether the directory exist.
    If Directory is not present it creates one and returns the path.
    """
    if not os.path.exists(dest):
      os.makedirs(dest)
    return dest
  
  def detect_filename(self, url, response):
    """
    Function to autodetect file name from url and content disposition
    headers.
    """
    if not self.filename == None:
      self.filename = self.get_valid_filename(self.filename)
    else:
      if 'filename' in response.headers.get('Content-Disposition'):
        filename = response.headers.get('Content-Disposition') \
          .split('filename=')[1].split(';')[0].replace('"', '')

      else:
        filename = os.path.basename(urlparse(unquote(response.url))[2])        

      self.filename = self.get_valid_filename(filename)
  
  def get_valid_filename(self, filename):
    """
    Return the given string converted to a string that can be used for a clean
    filename. Remove leading and trailing spaces; convert other spaces to
    underscores; and remove anything that is not an alphanumeric, dash,
    underscore, or dot.

    https://github.com/django/django/blob/master/django/utils/text.py
    """
    s = str(filename).strip()
    separator = ' '
    return re.sub(r'(?u)[^-\w.]', separator, s)
  
  def fix_existing_filename(self, filename, dest):
    """
    Function that checks whether the file is already downloaded(exists)
    If already downloaded adds a prefix of current timestamp and returns
    the filename along with proper extension
    """
    name, ext = filename.rsplit('.', 1)
    time = datetime.now().strftime('%m-%d-%Y_%I.%M.%S%p')
    name = name+'_'+time
    return name+'.'+ext
  
  def requests_retry_session(self,
    retries=3,
    backoff_factor=0.3,
    status_forcelist=(500, 502, 504),
    session=None,
    ):
    """
    A high level function that I certainly didnot write
    and I don't remember where I copied it from so if somebody knows whose code
    this is then inform me.

    What it bascially does is it automatically retries the request be it
    HEAD, POST, GET, DELETE for 3 times(defalut) can be changed.
    """
    session = session or requests.Session()
    retry = Retry(
        total=retries,
        read=retries,
        connect=retries,
        backoff_factor=backoff_factor,
        status_forcelist=status_forcelist,
    )
    adapter = HTTPAdapter(max_retries=retry)
    session.mount('http://', adapter)
    session.mount('https://', adapter)
    return session
  
  def download(self):
    """
    Function to download file into a temporary file and rename 
    it to user provided filename or autodetected filename.
    """
    try:
      with self.requests_retry_session().get(self.url, stream=True, timeout=3) as response:
        response.raise_for_status()
        self.detect_filename(self.url, response)
        self.file_size = int(response.headers['Content-Length'].strip())
        with open(os.path.join(self.dest, 'rget_'+str(uuid4())+'.tmp'), 'wb+') as temp:
          with tqdm(
            total = self.file_size,
            initial=0,
            unit='B',
            desc=self.filename,
            ascii=True,
            unit_scale=True,
            unit_divisor=1024,
          ) as progressBar:

            for chunk in response.iter_content(chunk_size=8192):
              temp.write(chunk)
              progressBar.update(len(chunk))

        if os.path.exists(os.path.join(self.dest, self.filename)):
          self.filename = self.fix_existing_filename(self.filename, self.dest)
        os.rename(temp.name, os.path.join(self.dest, self.filename))

      return self.filename

    #* A bit of Exception handling to showoff ;)
    except ReadTimeout:
        return('Maximum Retries reached, Check your internet connection and try again')
    
    except:
      return 'Please check the url and try again'

Usage:-

# importing Rget class from downloader.py
from downloader import Rget

url = 'https://drive.google.com/u/0/uc?id=18dn4ha9Lyb1MqjYEjtRAEA5uEKxjPkwD&export=download'

# Optional parameters like destination and fileName can also be provided
file = Rget(url = url)

# printing the fileName once the file gets downloaded
# since download funtion returns the filename
print(file.download())

Answer 1

Tout d'abord, quelques éléments de style / pelucheux:

1. Vous importez HTTPError et InvalidSchema à partir d'exceptions de demandes mais ne les utilisez pas.
2. Soyez cohérent au sujet de votre indentation. 4 espaces est le nombre recommandé par PEP8 et c'est OK si vous ne voulez pas suivre cela, mais essayez de ne pas mélanger les indentations de 2 et 4 espaces dans le même projet, comme vous le faites à l'intérieurrequests_retry_session()
3. Essayez d'utiliser le formatage de chaîne au lieu de concaténer avec +. Cela vous évite d'avoir à convertir manuellement les valeurs en str(comme vous le faites avec l'uuid in download()) et c'est également plus facile à lire. Jetez un œil aux chaînes f si vous êtes sur Python 3.6+ (ce que vous devriez):https://realpython.com/python-f-strings/
4. Ne comparez pas Noneavec ==. Utiliser le ismot-clé est la manière la plus idiomatique de le faire. La première ligne detect_filename()peut être réécrite comme if self.filename is not None. Voir:https://stackoverflow.com/questions/14247373/python-none-comparison-should-i-use-is-or
5. En règle générale, le code commenté est quelque chose dont nous n'avons pas besoin, nous pouvons donc aussi bien le supprimer complètement. Si jamais vous avez besoin de cette ligne, vous pouvez toujours la récupérer à partir de votre historique git. Parce que vous utilisez git, non? DROITE??

Choses mineures et pointilleuses:

1. La dernière partie de download()utilise une mise à part nue, ce qui est généralement une mauvaise idée car elle détecte certaines exceptions que vous ne voulez probablement pas attraper. Voir:https://stackoverflow.com/questions/54948548/what-is-wrong-with-using-a-bare-except
2. Votre docstring for fix_existing_filename()indique qu'il vérifie si le nom de fichier existe déjà, mais il ne le fait pas réellement.
3. Dans download(), vous n'avez pas besoin d'ouvrir le fichier en lecture-écriture si vous n'avez pas l'intention de le lire. Si vous définissez votre mode d'ouverture sur simplement, wbil est plus clair pour le lecteur que vous avez uniquement l'intention d'écrire dans ce fichier.
4. Dans check_if_dir_existvous n'avez pas besoin de l' ifinstruction, car vous pouvez passer exist_ok=Trueà os.makedirset cela créera automatiquement le répertoire uniquement s'il n'existe pas. En fait, je me débarrasserais complètement de cette méthode parce que vous pouvez tout faire en une seule ligne.
5. Au lieu de générer vous-même un nom de fichier temporaire, jetez un œil au tempfilemodule dans la bibliothèque standard. Non seulement cela résout le même problème que vous avez rencontré uuid4, mais il est également un peu plus clair pour le lecteur que vous générez un fichier temporaire. Voir:https://docs.python.org/3/library/tempfile.html#examples
6. requests_retry_session()prend un sessionargument pour permettre de réutiliser un argument existant requests.Session(), mais a) vous n'utilisez jamais cet argument et b) cela n'a pas beaucoup de sens. En tant que lecteur, je m'attendrais à ce qu'une fonction comme celle-ci crée une nouvelle session à chaque fois. Si la reconfiguration d'une session existante fait partie de la portée de cette fonction, elle devrait l'indiquer dans le nom d'une manière ou d'une autre.
7. Aussi à propos des sessions, c'est une bonne pratique d'installer un hook dessus afin qu'il appelle automatiquement raise_for_status()après chaque requête. De cette façon, vous n'avez pas à vous souvenir de le faire manuellement après chaque appel. La syntaxe peut sembler un peu étrange mais cela en vaut vraiment la peine:https://stackoverflow.com/questions/45470226/requests-always-call-raise-for-status
8. L'utilisation de detect_filename()est un peu bizarre. Je m'attendrais à ce qu'une méthode comme celle-ci renvoie le nom de fichier au lieu de mettre à jour l' filenameattribut et de ne rien rendre.

Des trucs plus gros:

1. Évitez de passer des appels dans les paramètres par défaut des fonctions, comme vous le faites dans __init__. L'appel n'est effectué qu'une seule fois au moment de la définition de la méthode et y est stocké pour toujours. Alors que, dans ce cas, votre cwdest toujours le même parce que vous ne changez pas votre répertoire actuel ailleurs, c'est un anti-modèle de faire des choses comme ça en Python. Cela semble étrange et vous pouvez également obtenir des résultats inattendus si vous ajoutez un chdirquelque part, car le résultat d'origine getcwd()sera toujours la fonction par défaut. Au lieu de cela, vous devez changer le desten Nonedans la définition de la méthode, puis ajouter un à l' if dest is None: dest = os.getcwd()intérieur.
2. Jetez un œil au pathlibmodule dans la bibliothèque standard. Il peut vous aider à simplifier la plupart de vos opérations de gestion de fichiers impliquant oset des os.pathappels. Il est également plus robuste car indépendant de la plate-forme. Voir:https://docs.python.org/3/library/pathlib.html
3. Certaines des méthodes de la classe ne sont pas du tout liées à la classe. get_valid_filename, fix_existing_filenameet requests_retry_sessionne jamais utiliser self, donc cela n'a pas beaucoup de sens pour eux d'être à l'intérieur de la classe. Au lieu de cela, vous devez extraire ces méthodes et en faire des fonctions. Si vous voulez vraiment qu'ils soient dans une classe, utilisez- @staticmethodles pour qu'il soit clair qu'ils n'interagissent pas avec la classe ou ses attributs, mais je recommanderais la première option.
4. Ce serait une bonne idée de stocker votre requests.Sessioncomme attribut, de sorte que vous n'ayez pas à le recréer à chaque fois que vous appelez download(). L'intérêt d'avoir une session est de pouvoir la réutiliser pour en profiter en enregistrant des cookies et en gardant les connexions ouvertes.
5. Dans download(), vous définissez file_sizecomme nouvel attribut, mais cela n'a pas beaucoup de sens. Avez-vous besoin de cela pour être un attribut? Est-ce une propriété de votre objet? Aurez-vous besoin de l'utiliser en dehors de la méthode actuelle? Si la réponse à toutes ces questions est «non», conservez-la plutôt comme variable locale.

Bonne choses:

1. Bonne séparation de votre logique entre plusieurs méthodes bien définies.
2. Docstrings informatifs, les gens ont tendance à les ignorer assez souvent.
3. tqdm! C'est une bibliothèque géniale et vous en tirez le meilleur parti en spécifiant correctement des éléments tels que les unités et la mise à l'échelle.
4. Une certaine gestion des exceptions est certainement mieux que rien. Ce n'est absolument pas une démonstration, mais quelque chose d'important à garder à l'esprit :)
5. Dans l'ensemble, c'est un bon code! Ne soyez pas découragé par le nombre de commentaires ici. Vous l'avez soumis à cette communauté, donc j'ai été tatillon exprès, mais ce code est meilleur que la plupart de ce que je lis au travail tous les jours :)