Downloader in Python

Question

Ich habe diesen Code in Python geschrieben und er funktioniert gut für mich, aber ich weiß, dass der Code nicht optimiert ist und viel Refactoring durchgeführt werden muss. Ich muss also überprüfen, wie dieser Code verbessert werden kann. Ich fing an, dies zu schreiben, als die WGET- Bibliothek für mich nicht funktionierte und ich ein leichtes Skript für meine anderen Projekte wollte. Ich denke auch darüber nach, die Anforderungsbibliothek durch aiohttp zu ersetzen. Als Anfänger freue ich mich auf Ihre Bewertungen.

Dankeschön.

Anforderungen : - tqdm , Anfragen

Implementierung: - 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'

Verwendung:-

# 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

Zunächst ein paar Stil- / Flusensachen:

1. Sie importieren HTTPError und InvalidSchema aus request.exceptions, verwenden sie jedoch nicht.
2. Seien Sie konsequent in Bezug auf Ihre Einrückung. 4 Leerzeichen ist die von PEP8 empfohlene Zahl und es ist in Ordnung, wenn Sie dem nicht folgen möchten, aber versuchen Sie nicht, Einrückungen mit 2 und 4 Leerzeichen im selben Projekt zu mischen, wie Sie es im Inneren tunrequests_retry_session()
3. Versuchen Sie, die Zeichenfolgenformatierung zu verwenden, anstatt sie zu verketten +. Dies erspart Ihnen die manuelle Konvertierung von Werten in str(wie bei der UUID in download()) und ist auch leichter zu lesen. Werfen Sie einen Blick auf F-Strings, wenn Sie Python 3.6+ verwenden (was Sie sollten):https://realpython.com/python-f-strings/
4. Nicht vergleichen Nonemit ==. Die Verwendung des isSchlüsselworts ist die idiomatischere Methode. Die erste Zeile in detect_filename()kann umgeschrieben werden als if self.filename is not None. Sehen:https://stackoverflow.com/questions/14247373/python-none-comparison-should-i-use-is-or
5. Kommentierter Code ist in der Regel etwas, das wir nicht benötigen. Sie können ihn also genauso gut vollständig löschen. Wenn Sie diese Zeile jemals wieder benötigen, können Sie sie jederzeit aus Ihrer Git-Historie abrufen. Weil du Git benutzt, richtig? RECHT??

Kleinere, pingelige Dinge:

1. Das letzte Stück download()verwendet eine bloße Ausnahme, was normalerweise eine schlechte Idee ist, da es einige Ausnahmen abfängt, die Sie wahrscheinlich nicht abfangen möchten. Sehen:https://stackoverflow.com/questions/54948548/what-is-wrong-with-using-a-bare-except
2. Ihre Dokumentzeichenfolge für fix_existing_filename()sagt, dass sie prüft, ob der Dateiname bereits vorhanden ist, dies jedoch nicht.
3. In download()müssen Sie die Datei nicht als Lese- / Schreibzugriff öffnen, wenn Sie nicht beabsichtigen, tatsächlich daraus zu lesen. Wenn Sie den Öffnungsmodus auf wbeinstellen, wird dem Leser nur klarer, dass Sie nur in diese Datei schreiben möchten.
4. In check_if_dir_existSie müssen die nicht - ifAnweisung, da Sie weitergeben können exist_ok=Truean os.makedirsund das wird das Verzeichnis nur dann automatisch erstellen , wenn es nicht existiert. In der Tat würde ich diese Methode vollständig loswerden, weil Sie einfach alles in einer Zeile tun können.
5. Schauen Sie sich das tempfileModul in der Standardbibliothek an, anstatt selbst einen temporären Dateinamen zu generieren . Es löst nicht nur das gleiche Problem, mit dem Sie gearbeitet haben uuid4, sondern es ist auch für den Leser etwas klarer, dass Sie eine temporäre Datei generieren. Sehen:https://docs.python.org/3/library/tempfile.html#examples
6. requests_retry_session()nimmt ein sessionArgument, um die Wiederverwendung eines vorhandenen zu ermöglichen requests.Session(), aber a) Sie verwenden dieses Argument nie und b) es macht nicht viel Sinn. Als Leser würde ich erwarten, dass eine solche Funktion jedes Mal eine neue Sitzung erstellt. Wenn die Neukonfiguration einer vorhandenen Sitzung Teil des Funktionsumfangs ist, sollte dies im Namen irgendwie angegeben werden.
7. Auch in Bezug auf die Sitzungen empfiehlt es sich, einen Hook zu installieren, damit er raise_for_status()nach jeder Anforderung automatisch aufgerufen wird. Auf diese Weise müssen Sie nicht daran denken, dies nach jedem Aufruf manuell zu tun. Die Syntax mag etwas seltsam aussehen, aber es lohnt sich auf jeden Fall:https://stackoverflow.com/questions/45470226/requests-always-call-raise-for-status
8. Die Verwendung von detect_filename()ist ein bisschen komisch. Ich würde erwarten, dass eine solche Methode den Dateinamen filenamezurückgibt, anstatt das Attribut zu aktualisieren und nichts zurückzugeben.

Größeres Zeug:

1. Vermeiden Sie es, Anrufe in Funktionsstandards zu tätigen, wie Sie es in tun __init__. Der Aufruf wird nur einmal zur Zeit der Methodendefinition ausgeführt und dort für immer gespeichert. In diesem Fall ist Ihr cwdVerzeichnis zwar immer dasselbe, da Sie Ihr aktuelles Verzeichnis nirgendwo anders ändern. Es ist jedoch ein Antimuster, solche Dinge in Python auszuführen. Es sieht seltsam aus und Sie können auch unerwartete Ergebnisse erhalten, wenn Sie jemals chdirirgendwo etwas hinzufügen , da das ursprüngliche Ergebnis getcwd()weiterhin die Standardeinstellung der Funktion ist. Stattdessen sollten Sie das destto Nonein der Methodendefinition ändern und dann ein if dest is None: dest = os.getcwd()darin hinzufügen .
2. Schauen Sie sich das pathlibModul in der Standardbibliothek an. Es kann Ihnen helfen, die meisten Dateiverwaltungsvorgänge zu vereinfachen, die Sie betreffen osund os.pathaufrufen. Es ist auch robuster, weil es plattformunabhängig ist. Sehen:https://docs.python.org/3/library/pathlib.html
3. Einige der Methoden in der Klasse sind überhaupt nicht mit der Klasse verbunden. get_valid_filename, fix_existing_filenameUnd requests_retry_sessionnie verwenden self, so dass es nicht viel Sinn machen für sie innerhalb der Klasse zu sein. Stattdessen sollten Sie diese Methoden extrahieren und funktionsfähig machen. Wenn Sie wirklich möchten, dass sie in einer Klasse sind, verwenden Sie @staticmethodsie, damit klar ist, dass sie nicht mit der Klasse oder ihren Attributen interagieren, aber ich würde die erste Option empfehlen.
4. Es wäre eine gute Idee, Ihr requests.SessionAttribut zu speichern , damit Sie es nicht jedes Mal neu erstellen müssen, wenn Sie es aufrufen download(). Der springende Punkt bei einer Sitzung ist, sie wiederverwenden zu können, um Cookies zu speichern und Verbindungen offen zu halten.
5. In setzen download()Sie file_sizeals neues Attribut, aber das macht nicht viel Sinn. Benötigen Sie das als Attribut? Ist es eine Eigenschaft Ihres Objekts? Müssen Sie es jemals außerhalb der aktuellen Methode verwenden? Wenn die Antwort auf all diese Fragen "Nein" lautet, behalten Sie sie stattdessen als lokale Variable bei.

Schöne Dinge:

1. Gute Trennung Ihrer Logik über mehrere genau definierte Methoden.
2. Informative Dokumente, die Leute neigen dazu, diese ziemlich oft zu überspringen.
3. tqdm! Es ist eine großartige Bibliothek, und Sie können das Beste daraus machen, indem Sie Dinge wie Einheiten und Skalierung richtig angeben.
4. Einige Ausnahmebehandlungen sind definitiv besser als nichts. Total kein Showoff, aber etwas Wichtiges zu beachten :)
5. Insgesamt ist es guter Code! Lassen Sie sich von der Anzahl der Kommentare hier nicht entmutigen. Du hast es dieser Community übermittelt, also war ich absichtlich pingelig, aber dieser Code ist besser als das meiste, was ich jeden Tag bei der Arbeit lese :)