jdalbey / libbyfetch

A Python program for downloading MP3 audiobooks from libbyapp.com website
MIT License
13 stars 5 forks source link

timeout_decorator.py use of signals not supported with windows #21

Closed manalive closed 1 day ago

manalive commented 2 days ago

`Message: module 'signal' has no attribute 'SIGALRM'

https://stackoverflow.com/questions/52779920/why-is-signal-sigalrm-not-working-in-python-on-windows

Tried substituting wrapt_timeout_decorator fork but it doesn't support TimeoutError.

jdalbey commented 2 days ago

Bummer. For the moment I suppose the workaround is to use the previous release in which timeout-decorator was not implemented.

jdalbey commented 1 day ago

@manalive Would you be willing to try out this updated version of libbyfetch.py that attempts to fix this issue with timeout-decorator? I created a branch in the repo for this issue. As described in e061a6288867428e6fc3fdccd8ddf4b3cc5be428 I'm discarding the timeout-decorator approach and instead check for browser installation using shutil which is cross-platform.

manalive commented 1 day ago

@jdalbey Thank you for your quick reply! I tried it and shutil.which() keeps returning false. Things I tried:

I also used the following simplified script to see if it was just the libbyfetch.py script and got the same results:

import shutil

def is_browser_installed_with_shutil(browser_name):
    """
    Checks if the specified browser (Chrome or Firefox) is installed using shutil.which().
    """
    # Define executable names for browsers
    executable_name = None
    if browser_name.lower() == "chrome":
        executable_name = "chrome.exe"
    elif browser_name.lower() == "firefox":
        executable_name = "firefox.exe"
    else:
        raise ValueError("Unsupported browser name. Use 'chrome' or 'firefox'.")

    # Use shutil.which to check if the executable is in the PATH
    return shutil.which(executable_name) is not None

if __name__ == "__main__":
    # Check for Chrome
    chrome_installed = is_browser_installed_with_shutil("chrome")
    print(f"Chrome installed: {chrome_installed}")

    # Check for Firefox
    firefox_installed = is_browser_installed_with_shutil("firefox")
    print(f"Firefox installed: {firefox_installed}")

I'm not a programmer, but I confirmed this sample an LLM spit out for me works:

import os
import shutil

def is_browser_installed(browser_name):
    """
    Checks if the specified browser (Chrome or Firefox) is installed.
    First checks the PATH using shutil.which(), then checks default installation paths.
    """
    # Define executable names for browsers
    executable_name = None
    if browser_name.lower() == "chrome":
        executable_name = "chrome.exe"
    elif browser_name.lower() == "firefox":
        executable_name = "firefox.exe"
    else:
        raise ValueError("Unsupported browser name. Use 'chrome' or 'firefox'.")

    # 1. Check if the executable is in the system's PATH
    if shutil.which(executable_name):
        return True

    # 2. Check default installation paths
    default_paths = {
        "chrome": [
            os.path.expandvars(r"%ProgramFiles%\Google\Chrome\Application\chrome.exe"),
            os.path.expandvars(r"%ProgramFiles(x86)%\Google\Chrome\Application\chrome.exe")
        ],
        "firefox": [
            os.path.expandvars(r"%ProgramFiles%\Mozilla Firefox\firefox.exe"),
            os.path.expandvars(r"%ProgramFiles(x86)%\Mozilla Firefox\firefox.exe")
        ]
    }

    for path in default_paths.get(browser_name.lower(), []):
        if os.path.exists(path):
            return True

    # If not found in PATH or default locations, return False
    return False

if __name__ == "__main__":
    # Check for Chrome
    chrome_installed = is_browser_installed("chrome")
    print(f"Chrome installed: {chrome_installed}")

    # Check for Firefox
    firefox_installed = is_browser_installed("firefox")
    print(f"Firefox installed: {firefox_installed}")

You could throw a path check in for windows users.

jdalbey commented 1 day ago

@manalive Thanks for testing this out and the suggested code revisions. I spent a little time playing with it and agree that this could work. My big reservation is that I'm very reluctant to write custom code for different OSes. In my experience it just leads to headaches down the road trying to maintain separate code for each platform. So for now I'm just going to scrap the whole idea of fixing #18. The prerequisites state that Chrome must be installed. I was taught that if a prerequisite isn't met that the outcome is indeterminate; thus having the application hang is technically expected behavior. :) I'll just make a note in the FAQ about it. Pragmatically most people will either have Chrome or be able to install it easily. Logistically I have more important priority issues to focus on.

jdalbey commented 1 day ago

Fixed in 191a5642fb8ffb3eec1099d83088cd7f7324218f. Removed timeout-decorator and reverted to omit check for installed browser.

manalive commented 1 day ago

The prerequisites state that Chrome must be installed. I was taught that if a prerequisite isn't met that the outcome is indeterminate; thus having the application hang is technically expected behavior. :)

Fair. Thanks!