Skip to content

Add ignore_repositories config for PR filtering #1736

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

miked-qc
Copy link

@miked-qc miked-qc commented Apr 30, 2025

User description

We want to enable the pr-agent for hundreds of projects in our organization, and then exclude a handful of repos from being reviewed. Given there seems to be no current solution for that, this option has been added here.

What Changed?

  • Added support to ignore PRs/MRs from specific repositories in GitHub, Bitbucket, and GitLab webhook logic
  • Updated configuration.toml to include ignore_repositories option
  • Added unit tests for ignore_repositories across all supported providers

PR Type

enhancement, tests


Description

  • Added ignore_repositories config to filter PRs/MRs by repository.

    • Implemented in GitHub, Bitbucket, and GitLab webhook logic.
    • Skips processing for PRs/MRs from listed repositories.
  • Updated configuration documentation to include ignore_repositories.

  • Added comprehensive unit tests for repository ignore logic.


Changes walkthrough 📝

Relevant files
Enhancement
bitbucket_app.py
Add repository ignore logic and debug logging for Bitbucket

pr_agent/servers/bitbucket_app.py

  • Added logic to skip PRs from repositories in ignore_repositories.
  • Added debug and info logging for repository filtering.
  • Improved debug output for other ignore conditions.
  • +16/-0   
    github_app.py
    Add repository ignore logic for GitHub PRs                             

    pr_agent/servers/github_app.py

  • Added logic to skip PRs from repositories in ignore_repositories.
  • Added info logging for repository filtering.
  • +8/-0     
    gitlab_webhook.py
    Add repository ignore logic for GitLab MRs                             

    pr_agent/servers/gitlab_webhook.py

  • Added logic to skip MRs from repositories in ignore_repositories.
  • Added info logging for repository filtering.
  • +8/-0     
    Documentation
    configuration.toml
    Document ignore_repositories config option                             

    pr_agent/settings/configuration.toml

    • Documented new ignore_repositories config option.
    +1/-0     
    Tests
    test_ignore_repositories.py
    Add tests for repository ignore filtering logic                   

    tests/unittest/test_ignore_repositories.py

  • Added unit tests for ignore_repositories logic across all providers.
  • Tests both matching and non-matching repository scenarios.
  • Tests behavior when config is empty.
  • +79/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • What Changed?
    * Added support to ignore PRs/MRs from specific repositories in GitHub, Bitbucket, and GitLab webhook logic
    * Updated configuration.toml to include ignore_repositories option
    * Added unit tests for ignore_repositories across all supported providers
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Debug Logging

    Multiple print statements with DEBUG prefix were added for debugging purposes. Consider replacing these with proper logger.debug() calls for consistency with the rest of the codebase.

        print(f"DEBUG: repo_full_name={repo_full_name}, ignore_repos={get_settings().get('CONFIG.IGNORE_REPOSITORIES', [])}")
        # logic to ignore PRs from specific repositories
        ignore_repos = get_settings().get("CONFIG.IGNORE_REPOSITORIES", [])
        if ignore_repos and repo_full_name:
            if repo_full_name in ignore_repos:
                print(f"DEBUG: Ignoring due to repo match: {repo_full_name}")
                get_logger().info(f"Ignoring PR from repository '{repo_full_name}' due to 'config.ignore_repositories' setting")
                return False
    
        # logic to ignore PRs from specific users
        ignore_pr_users = get_settings().get("CONFIG.IGNORE_PR_AUTHORS", [])
        if ignore_pr_users and sender:
            if sender in ignore_pr_users:
                print(f"DEBUG: Ignoring due to user match: {sender}")
                get_logger().info(f"Ignoring PR from user '{sender}' due to 'config.ignore_pr_authors' setting")
                return False
    
        # logic to ignore PRs with specific titles
        if title:
            ignore_pr_title_re = get_settings().get("CONFIG.IGNORE_PR_TITLE", [])
            if not isinstance(ignore_pr_title_re, list):
                ignore_pr_title_re = [ignore_pr_title_re]
            if ignore_pr_title_re and any(re.search(regex, title) for regex in ignore_pr_title_re):
                print(f"DEBUG: Ignoring due to title match: {title}")
                get_logger().info(f"Ignoring PR with title '{title}' due to config.ignore_pr_title setting")
                return False
    
        ignore_pr_source_branches = get_settings().get("CONFIG.IGNORE_PR_SOURCE_BRANCHES", [])
        ignore_pr_target_branches = get_settings().get("CONFIG.IGNORE_PR_TARGET_BRANCHES", [])
        if (ignore_pr_source_branches or ignore_pr_target_branches):
            if any(re.search(regex, source_branch) for regex in ignore_pr_source_branches):
                print(f"DEBUG: Ignoring due to source branch match: {source_branch}")
                get_logger().info(
                    f"Ignoring PR with source branch '{source_branch}' due to config.ignore_pr_source_branches settings")
                return False
            if any(re.search(regex, target_branch) for regex in ignore_pr_target_branches):
                print(f"DEBUG: Ignoring due to target branch match: {target_branch}")
                get_logger().info(
                    f"Ignoring PR with target branch '{target_branch}' due to config.ignore_pr_target_branches settings")
                return False
    except Exception as e:
        print(f"DEBUG: Exception in should_process_pr_logic: {e}")
        get_logger().error(f"Failed 'should_process_pr_logic': {e}")
    print("DEBUG: Returning True from should_process_pr_logic")

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Learned
    best practice
    Ensure list type safety

    The code assumes ignore_repos is always a list, but if
    CONFIG.IGNORE_REPOSITORIES returns a non-list value, the in check could fail.
    Add a type check to ensure ignore_repos is always treated as a list, similar to
    how ignore_pr_title_re is handled elsewhere in the code.

    pr_agent/servers/bitbucket_app.py [135-139]

     if ignore_repos and repo_full_name:
    +    if not isinstance(ignore_repos, list):
    +        ignore_repos = [ignore_repos]
         if repo_full_name in ignore_repos:
             print(f"DEBUG: Ignoring due to repo match: {repo_full_name}")
             get_logger().info(f"Ignoring PR from repository '{repo_full_name}' due to 'config.ignore_repositories' setting")
             return False
    • Apply this suggestion
    Suggestion importance[1-10]: 6
    Low
    Possible issue
    Fix inconsistent test payload

    The test creates a fixed body dictionary but then uses the body_func to generate
    the actual payload for each provider. This is inconsistent with the other tests
    and may lead to incorrect test behavior. Use the body_func directly with the
    repository name.

    tests/unittest/test_ignore_repositories.py [45-55]

     @pytest.mark.parametrize("provider_name, provider_func, body_func", PROVIDERS)
     def test_should_ignore_matching_repository(self, provider_name, provider_func, body_func):
         get_settings().set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
    -    body = {
    -        "pull_request": {},
    -        "repository": {"full_name": "org/repo-to-ignore"},
    -        "sender": {"login": "user"}
    -    }
    -    result = provider_func(body_func(body["repository"]["full_name"]))
    +    result = provider_func(body_func("org/repo-to-ignore"))
         print(f"DEBUG: Provider={provider_name}, test_should_ignore_matching_repository, result={result}")
         assert result is False, f"{provider_name}: PR from ignored repository should be ignored (return False)"
    • Apply this suggestion
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion correctly points out that the body variable in test_should_ignore_matching_repository is defined but only partially used, creating an inconsistency with the other tests in the file. The proposed change simplifies the code and improves consistency without changing the test's logic or outcome.

    Low
    • More
    • Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

    @mrT23
    Copy link
    Collaborator

    mrT23 commented May 4, 2025

    Thanks for the PR @miked-qc

    There are ample ways in Qodo Merge to disable automatic feedback on specific repos. (global and local configuration file, app repository selection)
    On PR-Agent there are less, I agree.

    Two issues should be addressed before I could approve the PR:

    1. Have you tried and tested these PR changes on the relevant git providers ? Specifically, are you sure this represents the actual payload received from the webhook ?
    2. Please change the logic of ignore_repositories to regex processing. It will also enable the opposite case - enabling pr-agent only on a specific repo, by using a 'not' trick

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

    Successfully merging this pull request may close these issues.

    2 participants