Python의 다운로더

Question

이 코드를 파이썬으로 작성했는데 잘 작동하지만 코드가 최적화되지 않았고 많은 리팩토링이 필요하다는 것을 알고 있습니다. 따라서이 코드를 개선 할 수있는 방법에 대한 검토가 필요합니다. 나는 WGET 라이브러리가 나를 위해 작동하지 않을 때 이것을 작성하기 시작했고 다른 프로젝트를 위해 가벼운 스크립트를 원했습니다. 또한 requests 라이브러리를 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. requests.exceptions에서 HTTPError 및 InvalidSchema를 가져 오지만 사용하지 않습니다.
2. 들여 쓰기에 대해 일관성을 유지하십시오. 4 개의 공백은 PEP8에서 권장하는 숫자이며이를 따르고 싶지 않아도 괜찮습니다.하지만 내부 에서처럼 동일한 프로젝트에서 2 개와 4 개의 공백 들여 쓰기를 혼합하지 마십시오.requests_retry_session()
3. 와 연결하는 대신 문자열 형식을 사용하십시오 +. 이렇게하면 수동으로 값을로 변환하는 수고를 str덜 수 있으며 (에서 uuid를 사용하는 것처럼 download()) 읽기도 더 쉽습니다. Python 3.6 이상을 사용하는 경우 f- 문자열을 살펴보십시오.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. 에서 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()만 a) 해당 인수를 절대 사용하지 않으며 b) 그다지 의미가 없습니다. 독자로서 저는 이와 같은 기능이 매번 새로운 세션을 생성 할 것으로 기대합니다. 기존 세션을 재구성하는 것이 해당 기능의 범위의 일부인 경우 이름에 어떻게 든이를 표시해야합니다.
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(). 세션을 갖는 요점은 쿠키를 저장하고 연결을 열린 상태로 유지하기 위해 세션을 재사용 할 수 있다는 것입니다.
5. 에서는 새 속성으로 download()설정 file_size했지만 그다지 의미가 없습니다. 속성이되기 위해 필요한가요? 객체의 속성입니까? 현재 방법 밖에서 사용해야합니까? 이들 모두에 대한 대답이 "아니오"이면 대신 지역 변수로 유지하십시오.

좋은 것들:

1. 잘 정의 된 여러 메서드에서 논리를 잘 분리합니다.
2. 유익한 독 스트링은 사람들이 자주 건너 뛰는 경향이 있습니다.
3. tqdm! 멋진 라이브러리이며 단위 및 크기 조정과 같은 항목을 올바르게 지정하여 최대한 활용할 수 있습니다.
4. 일부 예외 처리는없는 것보다 확실히 낫습니다. 완전히 과시는 아니지만 명심해야 할 중요한 사항 :)
5. 전반적으로 좋은 코드입니다! 여기에있는 댓글 수에 실망하지 마세요. 이 커뮤니티에 제출 했으므로 일부러 간결했지만이 코드는 내가 직장에서 매일 읽는 대부분의 것보다 낫습니다. :)