Skip to content

Ruff Check for all py files - #116

Merged
NiveditJain merged 16 commits into
FailproofAI:mainfrom
bhaveshAswani112:main
Jul 30, 2025
Merged

Ruff Check for all py files#116
NiveditJain merged 16 commits into
FailproofAI:mainfrom
bhaveshAswani112:main

Conversation

@bhaveshAswani112

Copy link
Copy Markdown
Contributor

This workflow now checks all changed .py files across the entire repository, instead of limiting the scan to only api-server/ and state-manager/ directories.
This update addresses issue #71.

@NiveditJain

Copy link
Copy Markdown
Member

@bhaveshAswani112 please check why CI is failing

@NiveditJain
NiveditJain self-requested a review July 30, 2025 08:29
@NiveditJain

Copy link
Copy Markdown
Member

@CodeRabbit

@NiveditJain

Copy link
Copy Markdown
Member

@@coderabbitai

@NiveditJain
NiveditJain merged commit 59e92ad into FailproofAI:main Jul 30, 2025
2 checks passed
@coderabbitai

coderabbitai Bot commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll help you investigate the CI failure. Let me check the workflow runs and examine the Ruff configuration.


🏁 Script executed:

#!/bin/bash
# Check recent workflow runs to see what's failing
gh run list --limit 10

# Look for GitHub Actions workflow files
fd -e yml -e yaml .github/workflows

# Check for Ruff configuration files
fd ruff.toml pyproject.toml .ruff.toml

# Look for any Python files that might have syntax issues
fd -e py | head -20

Length of output: 3082


Let me fix the commands and get more details about the CI failure:


🏁 Script executed:

#!/bin/bash
# Find GitHub Actions workflow files correctly
fd . .github/workflows

# Look for Ruff configuration files properly
fd -t f -e toml | grep -E "(ruff|pyproject)"
fd -t f -e yaml | grep ruff
fd -t f ".ruff.toml"

# Get details about the failed workflow run
gh run view 16614755036

# Check if there are any obvious Python syntax issues in recently changed files
git diff --name-only HEAD~5 | grep "\.py$" | head -10

Length of output: 1574


Now I can see the issue! Let me examine the workflow file and get the detailed failure logs:


🏁 Script executed:

#!/bin/bash
# Look at the ruff-check workflow file
cat .github/workflows/ruff-check.yml

# Get detailed logs of the failed run
gh run view 16614755036 --log-failed

# Check if the problematic file exists and has any issues
ls -la python-sdk/exospherehost/__init__.py || echo "File not found"

# Get recent commits to see what changed
git log --oneline -5

Length of output: 20534


@NiveditJain I found the issue! The CI is failing because of a problem with how the GitHub Actions workflow handles multiple changed Python files.

The Problem:
The workflow in .github/workflows/ruff-check.yml fails when there are multiple Python files changed. The error occurs in the "Get changed Python files" step:

##[error]Unable to process file command 'output' successfully.
##[error]Invalid format 'python-sdk/exospherehost/__init__.py'

Root Cause:
When git diff returns multiple files (separated by newlines), the workflow tries to set them as a GitHub Actions output using:

echo "files=$FILES" >> "$GITHUB_OUTPUT"

But GitHub Actions requires special formatting for multiline outputs. The current approach fails when $FILES contains multiple file paths.

The Fix:
The workflow needs to be updated to properly handle multiline outputs in GitHub Actions. Here's the corrected version of the "Get changed Python files" step:

- name: Get changed Python files
  id: changed-files
  run: |
    if [[ "${{ github.event_name }}" == "pull_request" ]]; then
      BASE_SHA=${{ github.event.pull_request.base.sha }}
      HEAD_SHA=${{ github.sha }}
    else
      BASE_SHA=${{ github.event.before }}
      HEAD_SHA=${{ github.event.after }}
    fi

    # List all changed .py files
    FILES=$(git diff --name-only --diff-filter=ACMRT $BASE_SHA $HEAD_SHA \
            | grep -E '\.py$' || true)

    # Handle multiline output properly for GitHub Actions
    if [[ -n "$FILES" ]]; then
      echo "files<<EOF" >> "$GITHUB_OUTPUT"
      echo "$FILES" >> "$GITHUB_OUTPUT"
      echo "EOF" >> "$GITHUB_OUTPUT"
    else
      echo "files=" >> "$GITHUB_OUTPUT"
    fi

And update the ruff command step to handle the files properly:

- name: Run Ruff on changed files
  if: steps.changed-files.outputs.files != ''
  run: |
    pip install ruff
    echo "Linting the following Python files:"
    echo "${{ steps.changed-files.outputs.files }}"
    # Convert newline-separated files to space-separated for ruff
    files="${{ steps.changed-files.outputs.files }}"
    ruff check $files

This fix uses GitHub Actions' heredoc syntax (<<EOF) to properly handle multiline outputs, which will resolve the CI failure.

@coderabbitai

coderabbitai Bot commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
pip install --quiet ruff
# Run Ruff on the whole repo and show the first 40 offences
ruff . | head -n 40

Length of output: 162


🏁 Script executed:

#!/bin/bash
set -e
pip install --quiet ruff
# Run Ruff check on the whole repo and show the first 40 violations
ruff check . | head -n 40

Length of output: 1088


@NiveditJain The CI is failing because the Ruff linter found 2 violations in Python files that weren't previously checked:

Issues found:

  1. python-sdk/exospherehost/node/__init__.py:1:23 - F401: BaseNode imported but unused

    • Fix: Use explicit re-export: from .BaseNode import BaseNode as BaseNode
  2. python-sdk/exospherehost/runtime.py:3:8 - F401: logging imported but unused

    • Fix: Remove the unused import logging line

Quick fix:
You can automatically fix the second issue by running:

ruff check . --fix

Since the workflow now scans all .py files (not just api-server/ and state-manager/), it discovered these linting violations in the python-sdk/ directory that need to be resolved for CI to pass.

@bhaveshAswani112

Copy link
Copy Markdown
Contributor Author

Hey @NiveditJain , sorry for the confusion, and thank you for merging the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants