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

bug-fix_azuredevops-new-file #1274

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

CoryBall
Copy link
Contributor

@CoryBall CoryBall commented Oct 8, 2024

User description

Azure Devops PRs with new files result in error "Failed to get diff files"

Summary

Issue: When pr-agent attempts to get the original content of a new file, a 404 is returned and pr-agent exits with an error.
Solution: Do not attempt to get original content if file edit_type is ADDED

Reproduce Issue:

  • Configure Azure Devops git provider
  • Create any PR with a new file that is NOT in the PR source branch
  • Run pr-agent "describe"/"review"/"improve"/etc

PR Type

Bug fix


Description

  • Fixed an issue where PR-agent would fail when processing Azure DevOps PRs containing new files
  • Added a check to skip retrieving original content for files with EDIT_TYPE.ADDED
  • Improved error handling by setting original_file_content_str to an empty string when content retrieval fails
  • This change prevents the "Failed to get diff files" error for PRs with new files in Azure DevOps

Changes walkthrough 📝

Relevant files
Bug fix
azuredevops_provider.py
Fix Azure DevOps PR handling for new files                             

pr_agent/git_providers/azuredevops_provider.py

  • Added a check for EDIT_TYPE.ADDED to skip retrieving original content
    for new files
  • Set original_file_content_str to an empty string for new files
  • Moved the original content retrieval logic into an else block
  • Added fallback to empty string if original content retrieval fails
  • +15/-12 

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

    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 8, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 95
    🧪 No relevant tests
    🔒 No security concerns identified
    🔀 No multiple PR themes
    ⚡ Recommended focus areas for review

    Error Handling
    The error handling for failed content retrieval could be improved by logging more specific error information.

    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 8, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Extract file content retrieval logic into a separate method for improved code organization

    Consider extracting the file content retrieval logic into a separate method to
    improve code readability and maintainability.

    pr_agent/git_providers/azuredevops_provider.py [339-354]

    -if edit_type == EDIT_TYPE.ADDED:
    -    original_file_content_str = ""
    -else:
    +original_file_content_str = self._get_original_file_content(file, version) if edit_type != EDIT_TYPE.ADDED else ""
    +
    +def _get_original_file_content(self, file, version):
         try:
    -        original_file_content_str = self.azure_devops_client.get_item(
    +        content = self.azure_devops_client.get_item(
                 repository_id=self.repo_slug,
                 path=file,
                 project=self.workspace_slug,
                 version_descriptor=version,
                 download=False,
                 include_content=True,
             )
    -        original_file_content_str = original_file_content_str.content
    +        return content.content
         except Exception as error:
             get_logger().error(f"Failed to retrieve original file content of {file} at version {version}", error=error)
    -        original_file_content_str = ""
    +        return ""
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: The suggestion to extract the file content retrieval logic into a separate method is valid and could improve code readability and maintainability. However, the PR has already made significant improvements to the code structure, so the impact of this change would be moderate rather than critical.

    5

    💡 Need additional feedback ? start a PR chat

    @mrT23
    Copy link
    Collaborator

    mrT23 commented Oct 8, 2024

    cool 👍

    @mrT23 mrT23 merged commit bb84063 into Codium-ai:main Oct 8, 2024
    @CoryBall CoryBall deleted the azure-devops-pr-new-file-error-fix branch November 1, 2024 17:54
    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