Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scripts to manage node launch #342

Merged
merged 7 commits into from
Sep 26, 2024
Merged

scripts to manage node launch #342

merged 7 commits into from
Sep 26, 2024

Conversation

DaMandal0rian
Copy link
Contributor

@DaMandal0rian DaMandal0rian commented Sep 25, 2024

PR Type

enhancement, documentation


Description

  • Added a shell script to install necessary Python dependencies and set up a virtual environment.
  • Documented the usage, setup, and features of the Subspace Node Manager in a README file.
  • Implemented a Python script to manage Subspace nodes using SSH and Docker Compose, including logging and error handling.
  • Added a TOML configuration file to specify server details for node deployment.

Changes walkthrough 📝

Relevant files
Enhancement
install_dependencies.sh
Add script to install dependencies and set up virtual environment

scripts/launch-nodes/install_dependencies.sh

  • Added a script to check for Python installation.
  • Created a virtual environment and installed dependencies.
  • Installed Python packages: paramiko, tomli, colorlog.
  • Provided instructions to activate the virtual environment.
  • +42/-0   
    manage_subspace.py
    Implement Subspace node management via SSH and Docker Compose

    scripts/launch-nodes/manage_subspace.py

  • Implemented SSH connection and command execution.
  • Managed Docker Compose operations for Subspace nodes.
  • Modified .env files and handled protocol version extraction.
  • Configured logging with color support.
  • +201/-0 
    Documentation
    README.md
    Add documentation for Subspace Node Manager usage and setup

    scripts/launch-nodes/README.md

  • Documented the Subspace Node Manager script features.
  • Provided installation and usage instructions.
  • Included logging and error handling details.
  • Added troubleshooting and license information.
  • +115/-0 
    Configuration changes
    nodes.toml
    Add TOML configuration for Subspace node deployment           

    scripts/launch-nodes/nodes.toml

  • Added configuration for bootstrap and farmer/RPC nodes.
  • Defined SSH connection details for each node.
  • +20/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 3 labels Sep 25, 2024
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Hardcoded Python Version
    The script uses python3 and pip3 commands which are hardcoded. This might not work as expected in environments where Python 3 is not the default Python version or is accessed via a different alias. Consider using python and pip and ensuring the script runs with Python 3.

    Error Handling
    The script lacks error handling after attempting to install virtualenv via pip. If the installation fails, the script will proceed without acknowledging the failure, which could lead to subsequent errors.

    Exception Handling
    Broad exception handling is used throughout the script. This can obscure the source of errors and make debugging more difficult. It's recommended to catch more specific exceptions where possible.

    Security Concern
    The script uses hardcoded paths and credentials in the TOML configuration file, which could expose sensitive information if the file is not properly secured.

    Copy link

    github-actions bot commented Sep 25, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Ensure consistent Python version usage by replacing pip with pip3

    Replace the use of pip with pip3 to ensure consistency and to avoid potential issues
    with the Python version. This change is necessary because the script explicitly uses
    Python 3 commands elsewhere, and using pip might accidentally use Python 2 if it is
    still installed on the system.

    scripts/launch-nodes/install_dependencies.sh [29]

    -pip install paramiko tomli colorlog
    +pip3 install paramiko tomli colorlog
     
    Suggestion importance[1-10]: 9

    Why: This suggestion is crucial for ensuring that the correct version of Python is used, as the script is explicitly using Python 3 commands. Using pip3 avoids potential issues with Python 2, which is a significant improvement.

    9
    Error handling
    Add error handling for the installation of virtualenv

    Add error handling for the pip3 install virtualenv command to ensure the script
    exits if the installation fails. This is crucial to prevent subsequent commands from
    running which depend on virtualenv being installed.

    scripts/launch-nodes/install_dependencies.sh [17]

    -pip3 install virtualenv
    +if ! pip3 install virtualenv; then
    +    echo "Failed to install virtualenv. Exiting..."
    +    exit 1
    +fi
     
    Suggestion importance[1-10]: 8

    Why: Adding error handling for the pip3 install virtualenv command is important to prevent the script from continuing when a critical dependency installation fails, improving robustness.

    8
    Ensure pip3 is installed before attempting to install virtualenv

    Add a check to ensure that pip3 is installed before attempting to install
    virtualenv. This prevents the script from failing without a clear error message if
    pip3 is not available.

    scripts/launch-nodes/install_dependencies.sh [15-18]

     if ! python3 -m venv --help &> /dev/null; then
         echo "virtualenv not found, installing..."
    -    pip3 install virtualenv
    +    if command -v pip3 &> /dev/null; then
    +        pip3 install virtualenv
    +    else
    +        echo "pip3 is not installed. Please install it first."
    +        exit 1
    +    fi
     fi
     
    Suggestion importance[1-10]: 8

    Why: Checking for pip3 before attempting to install virtualenv prevents the script from failing silently if pip3 is not installed, providing a clear error message and improving user experience.

    8
    Compatibility
    Enhance shell compatibility by using . instead of source

    Replace the use of source for activating the virtual environment with . to ensure
    compatibility with different shells that may not support source.

    scripts/launch-nodes/install_dependencies.sh [25]

    -source subspace_env/bin/activate
    +. subspace_env/bin/activate
     
    Suggestion importance[1-10]: 7

    Why: This change improves compatibility with different shells, which may not support source. While not critical, it enhances the script's portability.

    7

    dariolina
    dariolina previously approved these changes Sep 26, 2024
    Copy link
    Member

    @dariolina dariolina left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Tested yesterday (without POT), works wonders!

    @DaMandal0rian DaMandal0rian merged commit 7a3e6ac into main Sep 26, 2024
    1 check passed
    @DaMandal0rian DaMandal0rian deleted the launch-script branch September 26, 2024 10:22
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 3
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants