Загрузчик на Python

Question

Я написал этот код на python, и он отлично работает для меня, но я знаю, что код не оптимизирован, и необходимо провести большой рефакторинг. Поэтому мне нужен обзор того, как можно улучшить этот код. Я начал писать это, когда библиотека WGET у меня не работала, и я хотел облегчить сценарий для других моих проектов. Также подумываю заменить библиотеку запросов на aiohttp. Будучи новичком в этом, я с нетерпением жду ваших отзывов.

Благодарю вас.

Требования : - tqdm , запросы

Реализация: - 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'

Применение:-

# 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

Во-первых, пара вещей, связанных со стилем / линтингом:

1. Вы импортируете HTTPError и InvalidSchema из requests.exceptions, но не используете их.
2. Будьте последовательны в своем отступе. 4 пробела - это рекомендованное число в PEP8, и это нормально, если вы не хотите следовать этому, но постарайтесь не смешивать отступы 2 и 4 пробела в одном проекте, как вы делаете внутриrequests_retry_session()
3. Попробуйте использовать форматирование строки вместо объединения с +. Это избавляет вас от необходимости вручную преобразовывать значения в str(как вы это делаете с uuid in download()), а также его легче читать. Взгляните на f-строки, если вы используете Python 3.6+ (что вам следует):https://realpython.com/python-f-strings/
4. Не сравнивайте с Noneс ==. Использование isключевого слова - более идиоматический способ сделать это. Первую строку detect_filename()можно переписать как if self.filename is not None. Видеть:https://stackoverflow.com/questions/14247373/python-none-comparison-should-i-use-is-or
5. Как правило, прокомментированный код - это то, что нам не нужно, поэтому мы можем полностью удалить его. Если вам когда-нибудь понадобится эта строка, вы всегда можете получить ее из своей истории git. Потому что вы используете git, верно? ПРАВИЛЬНО??

Мелкие, придирчивые вещи:

1. Последний бит download()использует голое исключение, что обычно является плохой идеей, потому что он перехватывает некоторые исключения, которые вы, вероятно, не хотите ловить. Видеть:https://stackoverflow.com/questions/54948548/what-is-wrong-with-using-a-bare-except
2. В вашей строке документации fix_existing_filename()говорится, что он проверяет, существует ли уже имя файла, но на самом деле этого не делает.
3. В download(), вам не нужно открывать файл как чтение-запись, если вы не собираетесь читать из него. Установка режима открытия на просто wbпроясняет для читателя, что вы собираетесь писать только в этот файл.
4. In check_if_dir_existвам не нужен ifоператор, потому что вы можете перейти exist_ok=Trueк, os.makedirsи это автоматически создаст каталог, только если он не существует. Фактически, я бы полностью избавился от этого метода, потому что вы можете просто делать все в одной строке.
5. Вместо того, чтобы самостоятельно создавать временное имя файла, взгляните на tempfileмодуль в стандартной библиотеке. Он не только решает ту же проблему, с которой вы столкнулись uuid4, но и более ясно понимает, что вы создаете временный файл. Видеть:https://docs.python.org/3/library/tempfile.html#examples
6. requests_retry_session()принимает sessionаргумент, позволяющий повторно использовать существующий requests.Session(), но а) вы никогда не используете этот аргумент и б) это не имеет особого смысла. Как читатель, я ожидал бы, что такая функция будет каждый раз создавать новый сеанс. Если перенастройка существующего сеанса является частью области действия этой функции, тогда это должно каким-то образом указывать на это в имени.
7. Также по поводу сеансов, рекомендуется установить на него ловушку, чтобы он автоматически вызывал raise_for_status()после каждого запроса. Таким образом, вам не нужно помнить, что делать это вручную после каждого вызова. Синтаксис может показаться немного странным, но оно того стоит:https://stackoverflow.com/questions/45470226/requests-always-call-raise-for-status
8. Использование detect_filename()немного странное. Я ожидал, что такой метод вернет имя файла вместо обновления filenameатрибута и ничего не вернет.

Более крупный материал:

1. Избегайте вызовов функций по умолчанию, как в __init__. Вызов выполняется только один раз во время определения метода и сохраняется там навсегда. Хотя в этом случае ваш cwdвсегда один и тот же, потому что вы не меняете свой текущий каталог где-либо еще, это антипаттерн делать такие вещи в Python. Это выглядит странно, и вы также можете получить неожиданные результаты, если когда-нибудь добавите chdirгде-то, потому что исходный результат по- getcwd()прежнему будет функцией по умолчанию. Вместо этого, вы должны изменить , destчтобы Noneв определении метода , а затем добавьте if dest is None: dest = os.getcwd()в него.
2. Взгляните на pathlibмодуль в стандартной библиотеке. Это может помочь вам упростить большинство ваших операций управления файлами с участием osи os.pathвызовов. Он также более надежен, поскольку не зависит от платформы. Видеть:https://docs.python.org/3/library/pathlib.html
3. Некоторые методы в классе на самом деле вообще не связаны с классом. get_valid_filename, fix_existing_filenameи requests_retry_sessionникогда не использовать self, поэтому для них не имеет большого смысла находиться внутри класса. Вместо этого вы должны извлечь эти методы и сделать их функциями. Если вы действительно хотите, чтобы они были в классе, используйте @staticmethodих, чтобы было ясно, что они не взаимодействуют с классом или его атрибутами, но я бы рекомендовал первый вариант.
4. Было бы неплохо сохранить ваш requests.Sessionкак атрибут, чтобы вам не приходилось воссоздавать его каждый раз при вызове download(). Весь смысл сеанса состоит в том, чтобы иметь возможность повторно использовать его, чтобы воспользоваться преимуществами сохранения файлов cookie и сохранения соединений открытыми.
5. In download()вы устанавливаете file_sizeкак новый атрибут, но это не имеет большого смысла. Вам нужно, чтобы это было атрибутом? Это собственность вашего объекта? Вам когда-нибудь понадобится использовать его вне текущего метода? Если ответ на все эти вопросы - «нет», сохраните его как локальную переменную.

Приятные вещи:

1. Хорошее разделение вашей логики на несколько четко определенных методов.
2. Информационные строки документации, люди часто пропускают их.
3. tqdm! Это потрясающая библиотека, и вы можете максимально использовать ее, правильно определяя такие вещи, как единицы измерения и масштабирование.
4. Некоторая обработка исключений определенно лучше, чем ничего. Совершенно не показуха, но важно иметь в виду :)
5. В целом хороший код! Не расстраивайтесь из-за количества комментариев здесь. Вы отправили его этому сообществу, поэтому я специально придирался, но этот код лучше, чем большая часть того, что я читаю на работе каждый день :)