comfy_mtb icon indicating copy to clipboard operation
comfy_mtb copied to clipboard

[bug] requirements.txt often doesn't get read properly

Open Gerkinfeltser opened this issue 1 year ago • 2 comments

Describe the bug

The custom MTB node library was frequently failing due to issues with the requirements.txt file being read as pip module to install rather than a list of modules. This issue has been addressed by modifying the utils.py file to handle the installation of packages from requirements.txt more robustly.

Reproduction

  1. Start comfyui.
  2. Observe the startup log. 50% of the time mtb would fail due to improper handling of requirements.txt

Expected behavior

mtb loads without issue.

Operating System

Windows (Default)

Comfy Mode

Comfy Portable (embed) (Default)

Console output

No response

Additional context

I actually fixed it on my end with the following code changes (with the help of ChatGPT) The following block of code replaces lines 429 of utils.py to line 447. Essentially just after the print("Command executed successfully!") line up to just before the # endregion line.

    print("Command executed successfully!")

# Fix for requirements installation issue
def import_install(package_name):
import importlib
import subprocess
import sys
import os

def import_install(package_name):
    if package_name == "requirements":
        # Handle the special case for "requirements"
        requirements_file = os.path.join(os.path.dirname(__file__), 'requirements.txt')
        if os.path.exists(requirements_file):
            print(f"[comfy_mtb] 'requirements.txt' found. Installing packages from it...")
            try:
                # Suppress pip output by redirecting stdout and stderr
                result = subprocess.check_call(
                    stdout=subprocess.DEVNULL,  # Suppress stdout
                    stderr=subprocess.DEVNULL   # Suppress stderr
                )
                print(f"[comfy_mtb] All packages from 'requirements.txt' installed successfully.")
            except subprocess.CalledProcessError as e:
                print(f"[comfy_mtb] Failed to install packages from 'requirements.txt'. Error: {e}")
        else:
            print(f"[comfy_mtb] 'requirements.txt' not found in the directory.")
        return  # Exit the function after handling requirements.txt

    # For other packages, attempt to import and install if necessary
    try:
        importlib.import_module(package_name)
    except ModuleNotFoundError:
        print(f"[comfy_mtb] Package '{package_name}' not found. Installing...")
        try:
            # Suppress pip output by redirecting stdout and stderr
            subprocess.check_call(
                [sys.executable, "-m", "pip", "install", package_name],
                stdout=subprocess.DEVNULL,  # Suppress stdout
                stderr=subprocess.DEVNULL   # Suppress stderr
            )
            # After installing, try importing again
            importlib.import_module(package_name)
            print(f"[comfy_mtb] Package '{package_name}' successfully installed and imported.")
        except subprocess.CalledProcessError as e:
            print(f"[comfy_mtb] Failed to install package '{package_name}'. Error: {e}")
        except ModuleNotFoundError:
            print(f"[comfy_mtb] Package '{package_name}' could not be imported even after installation.")
# End Fix for requirements installation issue

# endregion

Changes Made:

  1. Special Handling for requirements.txt:
    • Added a special case in the import_install function to handle the requirements package.
    • If the requirements.txt file exists in the directory, the function attempts to install all packages listed in it.
    • The installation process suppresses pip output by redirecting stdout and stderr to subprocess.DEVNULL.
  2. Error Handling:
    • Added error handling to catch and log any errors that occur during the installation of packages from requirements.txt.
    • If the installation fails, an error message is printed with details about the failure.
  3. General Package Installation:
    • For other packages, the function attempts to import the package first.
    • If the import fails, it attempts to install the package using pip and then tries to import it again.
    • Added error handling for cases where the package cannot be imported even after installation.

Impact:

  • These changes should significantly reduce the frequency of failures related to the requirements.txt file in the custom MTB node library.
  • The new error handling and logging provide better visibility into what went wrong during package installation.
  • The output may be a bit too verbose for some & isn't heavily tested so I was hesitant to do a pull request.

Thanks so much for the node library! Comfy would be nearly unusable without it. :D

Gerkinfeltser avatar Oct 30 '24 15:10 Gerkinfeltser

Thanks but i'm not entirely following. I do not automatically handle dependencies anymore (the code you are refering to is never called it was back when I was shipping an install.py) so all this should be handled by Comfy Manager / Comfy CLI. That said there are maybe better approaches now for non manager users.

melMass avatar Nov 05 '24 14:11 melMass

Oh wait yes I still use import_install in a few places! But the goal is to only install the provided dependency not the entire requirements.txt, I'm away today but I will check your suggestion more closely tomorrow.

melMass avatar Nov 05 '24 14:11 melMass