2
\$\begingroup\$

Any help with my code. I think it's kind of hard code where I want to extract links from a page and download them.

It's going to scrape some links from websites (but the Scraper function that I've written is hardcode) and download the videos if they exist and I'm sure it's not good at all and it's more of a smell code.

Code

import requests
import subprocess
import getpass
import time
from bs4 import BeautifulSoup as bs
# import argparse # Add Command Line Arguments to a Python Script (switch_flag)


def Login(url, login_route, username, password):

    headers = {
        'User-Agent': '', 
        'origin': url,
        'referer': url + login_route,
    }

    request_session = requests.session()

    csrf_token = request_session.get(url).cookies['csrftoken']

    login_payload = {
        'hidden_username': username,
        'password': password,
        'csrfmiddlewaretoken': csrf_token
    }

    login_request = request_session.post(
        url + login_route, headers=headers, data=login_payload)


    if login_request.status_code == 200:
        msg = f'\nYou have logged in successfully {login_request.status_code}'
    else:
        msg = f'\nError {login_request.status_code}'

    print(msg)


def get_user_input():

    url = input('URL: ')
    username = getpass.getpass('USERNAME: ')
    password = getpass.getpass('PASSWORD: ')
    login_route = input('LOGIN_ROUTE: ')

    return Login(url, login_route, username, password)


def Scraper(page_url):

    headers = {
        ''
    }

    page = requests.get(
        page_url,
        headers=headers,
    )
    soup = bs(page.text, "html.parser")

    URL_List = []
    link_count = 0
    for a_tag in soup.select('a[href^="/course/"]'):

        links = "https://maktabkhooneh.org" + a_tag["href"]

        URL_List.append(links)
        link_count += 1

    return URL_List


def Donwloader(url_list):

    URL_List = Scraper(url_list)
    download_count = 0
    
    try:
        for links in URL_List:
            command = f'youtube-dl  {links}'
            result = subprocess.call(command, shell=True)
            if result == 0:
                download_count += 1

    except KeyboardInterrupt:
        
        print('Paused ;)')
            

    return f'\n{download_count} file(s) have been downloaded'


if __name__ == '__main__':

    page_url = input('Please enter the page URL: ')

    Login_permission = input('Login required Website [Y], [N]? ')
    if Login_permission == 'y' or Login_permission == 'Y':
        get_user_input()

    list_len = len(Scraper(page_url))
    Download_Permission = input(
        f'\n{list_len} link(s) have been extracted. Do you want to DOWNLOAD them [Y], [N]? ')

    Scraper(page_url)

    if Download_Permission == 'y' or Download_Permission == 'Y':
        Donwloader(page_url)
    else:
        print('\nProcess has been canceled ;)')
\$\endgroup\$
2
  • \$\begingroup\$ What is an example of a page URL that this can accept? \$\endgroup\$
    – Reinderien
    Jun 24 at 16:46
  • \$\begingroup\$ For example, maktabkhooneh.org/course/… \$\endgroup\$
    – Xus
    Jun 24 at 17:31
1
\$\begingroup\$

A few suggestions.

It's good that you are using a session but you are not using it properly. The idea is to reuse the session across your code to increase efficiency while crawling sites. It is used in the login routine but in the Scraper routine you are using a plain requests.get(). So you are not actually taking advantage of requests.session.

You should actually set a user agent in your headers. Not having one, or using the default value (something like: python-requests/2.25.1) makes it obvious that you are bot. Thus, your script may have trouble crawling some sites. I would suggest that you spoof the user agent to match a regular browser. It doesn't cost anything.

You are using F-strings in your code but sometimes concatenation using the + sign. For more consistency I suggest that you use F-strings only.

I would try to refactor your set of function as a class. Below is a quick attempt, it is not complete and meant for demonstration purposes. I have set a few headers in particular a spoofed user agent (Firefox). It is enough to do it once at the session level, since every request (get, post) that uses the session inherits from these parameters. So you don't have to repeat yourself. But you can still pass extra headers on a case by case basis, this is what is being done in the login route where we want to pass a referer for example.

I tried to add a bit of abstraction by creating a function to retrieve a single page. Then I would design another function to actually parse it using BS4 etc.

I also added some basic error handling at the application level, plus some logging on console, you can easily add logging to a file if desired (useful for troubleshooting).

You might want to use a configuration file to store URLs and other settings to avoid the hassle of typing in values. Using getpass is fine if you don't want to store credentials or other sensitive information.

PS: a discussion about using shell=True with subprocess: Actual meaning of 'shell=True' in subprocess. In short I don't think you need it, it may even be detrimental from a security point of view.

Since you are scrapping a third-party site, the HTML should be considered as unsafe, unsanitized, untrusted user input.

And if the call fails (return code != 0), don't stay silent but log this event. All you have to do is to take advantage of the logger already available and do something like:

    if result == 0:
        download_count += 1
    else:
        self.logger.warning(f"Failed to execute command: {command} - Return code: {result}")

Then at least you have a trace of the things that go wrong during the process.

Maybe that's the thing that is missing in your script: traceability. It's good to print some messages here and there, to show what the script is doing and at what stage it currently is. Especially when it's expected that execution will take a while, you want to be able to track progress. The logging module is there for you, use it and abuse it.


Misc: in Scraper, you increment link_count but you could just use len(URL_List) instead. No need for another variable.


PS: it would make sense to move the class to a standalone file, so that it can be reused and there is better separation. Just import it like another module. Then the main script will be smaller and more manageable.


Class skeleton

import sys
import logging
import requests
import getpass

from bs4 import BeautifulSoup as bs


class Downloader:

    def __init__(self, logger=None):
        # set up logging
        self.logger = logger or logging.getLogger(__name__)
        self.logger.debug("Initialize class Downloader")

        # init session
        self.session = requests.session()

        # add some standard headers and spoof user agent
        self.session.headers.update({
            "User-Agent": "Mozilla/5.0 (X11; Linux x86_64; rv:89.0) Gecko/20100101 Firefox/89.0",
            "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8",
            "Accept-Language": "en-US,en;q=0.5"
        })
        self.logger.debug(f"Session headers: {self.session.headers}")

    def fetch_page(self, url: str, headers: None):
        """Fetch a single page and return request results - some extra headers may be provided
        """
        self.logger.debug(f"Fetching page: {url}")
        return self.session.get(url=url, headers=headers)

    def login(self, url: str, csrf_token: str, username: str, password: str, headers: None):
        """Log in to website - some extra headers may be provided
        """
        self.logger.debug(f"Logging in with user: {username}")
        login_payload = {
            'hidden_username': username,
            'password': password,
            'csrfmiddlewaretoken': csrf_token
        }
        return self.session.post(url=url, data=login_payload, headers=headers)


def main():

    try:
        # set up simple logging to console
        logging.getLogger().setLevel(logging.NOTSET)

        logger = logging.getLogger(__name__)

        # Add stdout handler, with level DEBUG
        console = logging.StreamHandler(sys.stdout)
        console.setLevel(logging.DEBUG)
        formatter = logging.Formatter(
            '%(asctime)s - %(levelname)s - %(message)s',
            datefmt="%Y-%m-%d %H:%M:%S"
        )
        console.setFormatter(formatter)
        logger.addHandler(console)

        # get URL and credentials
        url = input('URL: ')
        login_route = input('LOGIN_ROUTE: ')
        username = getpass.getpass('USERNAME: ')
        password = getpass.getpass('PASSWORD: ')

        # start crawling
        downloader = Downloader(logger=logger)
        page = downloader.fetch_page(url=url)
        if page.status_code == 200:
            # fetch the CSRF token - return None if not found
            csrf_token = page.cookies.get('csrftoken')
            if csrf_token is None:
                raise Exception("Failed to obtain the CSRF token")
            else:
                logger.debug(f"Got CSRF token: {csrf_token}")

            # proceed to login - add some extra headers on top of the session headers
            login_headers = {
                'origin': url,
                'referer': url + login_route,
            }
            login_request = downloader.login(
                url=login_route, csrf_token=csrf_token, username=username, password=password,
                headers=login_headers
            )
            if login_request.status_code == 200:
                logger.debug(f"You have logged in successfully - status code: {page.status_code}")
            else:
                raise Exception(f"Failed to login - status code: {login_request.status_code}")

            # TODO: loop on links and retrieve videos
        else:
            raise Exception(f"Failed to fetch URL: {url} - status code: {page.status_code}")


    # Ctrl-C
    except KeyboardInterrupt:
        logger.warning("Shutdown requested (KeyboardInterrupt)...")
        sys.exit(0)

    except Exception:
        logger.error("Exception occured", exc_info=True)
        sys.exit(1)
    finally:
        logger.debug("Application shutting down")


if __name__ == '__main__':
    main()
\$\endgroup\$
1
  • \$\begingroup\$ thank you so much. But how can I pass the login part in case the site is not login required? \$\endgroup\$
    – Xus
    Jun 25 at 5:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.