Downloader em Python

Question

Eu escrevi este código em python e ele funciona bem para mim, mas eu sei que o código não é otimizado e muita refatoração precisa ser feita. Portanto, preciso revisar como esse código pode ser melhorado. Comecei a escrever isso quando a biblioteca WGET não estava funcionando para mim e eu queria um script leve para meus outros projetos. Também estou pensando em substituir a biblioteca de solicitações por aiohttp. Sendo um iniciante nisso, aguardo seus comentários.

Obrigado.

Requisitos : - tqdm , solicitações

Implementação: - 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'

Uso:-

# 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

Primeiro, algumas coisas de estilo / linting:

1. Você está importando HTTPError e InvalidSchema de requests.exceptions, mas não os está usando.
2. Seja consistente com sua indentação. 4 espaços é o número recomendado pelo PEP8 e está tudo bem se você não quiser seguir isso, mas tente não misturar recuo de espaço 2 e 4 no mesmo projeto, como você está fazendo por dentrorequests_retry_session()
3. Tente usar formatação de string em vez de concatenar com +. Isso evita o trabalho de converter manualmente os valores para str(como você faz com o uuid em download()) e também é mais fácil de ler. Dê uma olhada nas strings f se você estiver no Python 3.6+ (o que deve ser feito):https://realpython.com/python-f-strings/
4. Não compare Nonecom ==. Usar a ispalavra-chave é a maneira mais idiomática de fazer isso. A primeira linha detect_filename()pode ser reescrita como if self.filename is not None. Vejo:https://stackoverflow.com/questions/14247373/python-none-comparison-should-i-use-is-or
5. Como regra geral, o código comentado é algo de que não precisamos, portanto, podemos também excluí-lo completamente. Se você precisar dessa linha de volta, você sempre pode obtê-la de seu histórico do git. Porque você está usando git, certo? DIREITO??

Coisas menores e minuciosas:

1. O último bit de download()usa uma exceção nua, o que normalmente é uma má ideia porque captura algumas exceções que você provavelmente não deseja capturar. Vejo:https://stackoverflow.com/questions/54948548/what-is-wrong-with-using-a-bare-except
2. Seu docstring para fix_existing_filename()diz que verifica se o nome do arquivo já existe, mas na verdade não faz isso.
3. Em download(), você não precisa abrir o arquivo como leitura e gravação se não pretende realmente lê-lo. Definir o modo aberto para apenas wbtorna mais claro para o leitor que você pretende gravar apenas naquele arquivo.
4. Em check_if_dir_existvocê não precisa da ifinstrução, porque você pode passar exist_ok=Truepara os.makedirse isso criará automaticamente o diretório apenas se ele não existir. Na verdade, eu me livraria totalmente desse método porque você pode fazer tudo em uma linha.
5. Em vez de você mesmo gerar um nome de arquivo temporário, dê uma olhada no tempfilemódulo na biblioteca padrão. Não só resolve o mesmo problema que você fez uuid4, mas também fica um pouco mais claro para o leitor que você está gerando um arquivo temporário. Vejo:https://docs.python.org/3/library/tempfile.html#examples
6. requests_retry_session()leva um sessionargumento para permitir a reutilização de um existente requests.Session(), mas a) você nunca usa esse argumento eb) não faz muito sentido. Como leitor, espero que uma função como essa crie uma nova sessão a cada vez. Se reconfigurar uma sessão existente faz parte do escopo dessa função, então deve indicar isso de alguma forma no nome.
7. Também sobre as sessões, é uma boa prática instalar um gancho para que ele chame automaticamente raise_for_status()após cada solicitação. Dessa forma, você não precisa se lembrar de fazer isso manualmente após cada invocação. A sintaxe pode parecer um pouco estranha, mas definitivamente vale a pena:https://stackoverflow.com/questions/45470226/requests-always-call-raise-for-status
8. O uso de detect_filename()é um pouco estranho. Eu esperaria que um método como esse retornasse o nome do arquivo em vez de atualizar o filenameatributo e não devolver nada.

Coisas maiores:

1. Evite fazer chamadas em padrões de função, como você faz em __init__. A chamada é realizada apenas uma vez no momento da definição do método e armazenada lá para sempre. Embora, neste caso, o seu cwdseja sempre o mesmo porque você não está alterando seu diretório atual em nenhum outro lugar, é um antipadrão fazer coisas como essa em Python. Parece estranho e você também pode obter resultados inesperados se adicionar um em chdiralgum lugar, porque o resultado original getcwd()ainda será o padrão da função. Em vez disso, você deve alterar o destpara Nonena definição do método e, em seguida, adicionar um if dest is None: dest = os.getcwd()dentro dele.
2. Dê uma olhada no pathlibmódulo na biblioteca padrão. Pode ajudá-lo a simplificar a maioria das operações de gerenciamento de arquivos envolvendo ose os.pathchamadas. Também é mais robusto porque é independente de plataforma. Vejo:https://docs.python.org/3/library/pathlib.html
3. Alguns dos métodos da classe não estão realmente relacionados à classe. get_valid_filename, fix_existing_filenamee requests_retry_sessionnunca use self, então não faz muito sentido para eles estarem dentro da classe. Em vez disso, você deve extrair esses métodos e torná-los funções. Se você realmente deseja que eles participem de uma aula, use @staticmethodneles para que fique claro que eles não interagem com a classe ou seus atributos, mas eu recomendaria a primeira opção.
4. Seria uma boa ideia armazená-lo requests.Sessioncomo um atributo, para que você não precise recriá-lo sempre que invocar download(). O objetivo de ter uma sessão é ser capaz de reutilizá-la para aproveitar as vantagens de salvar cookies e manter as conexões abertas.
5. Em download(), você definiu file_sizecomo um novo atributo, mas isso não faz muito sentido. Você precisa que isso seja um atributo? É uma propriedade do seu objeto? Você precisará usá-lo fora do método atual? Se a resposta para todas essas perguntas for "não", mantenha-a como uma variável local.

Coisas legais:

1. Boa separação de sua lógica em vários métodos bem definidos.
2. Docstrings informativos, as pessoas tendem a ignorá-los com frequência.
3. tqdm! É uma biblioteca incrível e você aproveita ao máximo especificando coisas como unidades e escala.
4. Algum tratamento de exceção é definitivamente melhor do que nada. Totalmente não um showoff, mas algo importante para se manter em mente :)
5. No geral, é um bom código! Não desanime pelo número de comentários aqui. Você o enviou a esta comunidade, então fui minucioso de propósito, mas este código é melhor do que a maioria do que leio no trabalho todos os dias :)