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

Add --version command and version metadata #1384

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MarkRx
Copy link
Contributor

@MarkRx MarkRx commented Dec 6, 2024

User description

Implements #1370

Checks version info from pyproject.toml if running locally* before checking for pip metadata.

  • Uses tomlib which was added in 3.11. Will return "unknown" if using 3.10. The other option would be to bump the minimum version to 3.11 or else import a 3rd party toml library

PR Type

enhancement


Description

  • Added a get_version function to retrieve version information from pyproject.toml or pip metadata.
  • Implemented a --version command in the CLI to display the current version of the application.
  • Enhanced logging by including version metadata in the tags for better traceability.
  • Updated the project version in pyproject.toml to 0.2.6.

Changes walkthrough 📝

Relevant files
Enhancement
litellm_ai_handler.py
Add version metadata to logging tags                                         

pr_agent/algo/ai_handlers/litellm_ai_handler.py

  • Imported get_version from pr_agent.version.
  • Added version metadata to tags in capture_logs function.
  • +3/-2     
    cli.py
    Implement --version command in CLI                                             

    pr_agent/cli.py

  • Imported get_version from pr_agent.version.
  • Added --version command to CLI.
  • +2/-0     
    version.py
    Add version retrieval utility                                                       

    pr_agent/version.py

  • Created get_version function to retrieve version from pyproject.toml
    or pip metadata.
  • +20/-0   
    Configuration changes
    pyproject.toml
    Update project version in pyproject.toml                                 

    pyproject.toml

    • Updated project version to 0.2.6.
    +1/-1     

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

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    1370 - Partially compliant

    Compliant requirements:

    • Add way to programically obtain metadata from pip/pyproject.toml
    • Add version metadata to langchain context
    • Add version command

    Non-compliant requirements:

    • Optionally add version metadata to pr agent accepted suggestions
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 92
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling
    The error handling for Python versions below 3.11 could be improved. Currently it silently falls back to pip version check without logging any warning about tomllib unavailability.

    Path Resolution
    The pyproject.toml path is checked relative to current directory. This might fail if the script is run from a different directory. Consider using absolute path resolution.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add error handling for missing or malformed configuration data to prevent runtime crashes

    Add error handling for the case when pyproject.toml exists but is malformed or
    missing the version field. Currently, this would raise a KeyError.

    pr_agent/version.py [11-13]

     with open("pyproject.toml", "rb") as f:
         data = tomllib.load(f)
    -    return data["project"]["version"]
    +    try:
    +        return data["project"]["version"]
    +    except KeyError:
    +        get_logger().warning("Version not found in pyproject.toml")
    +        return "unknown"
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This is a critical error handling improvement that prevents crashes when pyproject.toml exists but has missing or invalid version data, which could affect the application's stability in production.

    8
    Add error handling for file system operations to prevent application crashes

    Handle potential IOError/OSError when trying to read pyproject.toml file to prevent
    crashes if the file is inaccessible.

    pr_agent/version.py [9-11]

     if os.path.exists("pyproject.toml") and sys.version_info >= (3, 11):
         import tomllib
    -    with open("pyproject.toml", "rb") as f:
    +    try:
    +        with open("pyproject.toml", "rb") as f:
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Important defensive programming suggestion that handles potential file system errors when reading pyproject.toml, preventing crashes in case of file permission or access issues.

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

    💡 Need additional feedback ? start a PR chat

    @MarkRx MarkRx force-pushed the feature/version-metadata branch from 5a187a9 to 26a5f43 Compare December 9, 2024 16:27
    pyproject.toml Outdated
    @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

    [project]
    name = "pr-agent"
    version = "0.2.4"
    version = "0.2.6"
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    the current version is 0.2.5

    Comment on lines 1 to 23
    import os
    import sys
    from importlib.metadata import version, PackageNotFoundError

    from pr_agent.log import get_logger

    def get_version() -> str:
    # First check pyproject.toml if running directly out of repository
    if os.path.exists("pyproject.toml"):
    if sys.version_info >= (3, 11):
    import tomllib
    with open("pyproject.toml", "rb") as f:
    data = tomllib.load(f)
    return data["project"]["version"]
    else:
    get_logger().error("Unable to determine local version from pyproject.toml")

    # Otherwise get the installed pip package version
    try:
    return version('pr-agent')
    except PackageNotFoundError:
    get_logger().error("Unable to find package named 'pr-agent'")
    return "unknown"
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    add this to an existing util file. no need for new file

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    Which one should it go in?

    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    @mrT23
    Copy link
    Collaborator

    mrT23 commented Dec 11, 2024

    looks good. after fixing the feedback, can be merged

    @MarkRx MarkRx force-pushed the feature/version-metadata branch from 26a5f43 to 452abe2 Compare December 17, 2024 16:04
    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