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

feat: set review data to github actions output #853

Merged
merged 8 commits into from
Apr 11, 2024

Conversation

idubnori
Copy link
Contributor

@idubnori idubnori commented Apr 9, 2024

User description

Add following feature under new GITHUB_ACTION_CONFIG.ENABLE_OUTPUT config.
Set review result data to github actions output parameter .

Usage Example

steps:
  - name: PR Agent action step
    id: pragent
    uses: Codium-ai/pr-agent@main
    env:
      OPENAI_KEY: ${{ secrets.OPENAI_KEY }}
      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      # Default is true
      # GITHUB_ACTION_CONFIG.ENABLE_OUTPUT: false
  - run: |
      # Yes or No
      echo "${{ fromJSON(steps.pragent.outputs.review).relevant_tests }}"

Note

  • default config value is true if use in github actions, otherwise false
  • Write documentation, if needed
  • More data output other than the review result would be in future PRs

Why

Convenient to be able to use the review result without parsing markdown formatted comment in next step action.


Type

enhancement, tests


Description

  • Implemented a feature to write review data to GitHub Actions output, controlled by GITHUB_ACTION_CONFIG.ENABLE_OUTPUT.
  • Added github_output function in utils.py for handling the output writing process.
  • Configured GITHUB_ACTION_CONFIG.ENABLE_OUTPUT setting in github_action_runner.py to enable or disable the output feature based on environment variables.
  • Utilized the github_output function in pr_reviewer.py to output review data.
  • Added unit tests in test_github_output.py to ensure the functionality works as expected under different scenarios (enabled, disabled, and unset).
  • Introduced enable_output configuration in configuration.toml to manage this feature through settings.

Changes walkthrough

Relevant files
Enhancement
utils.py
Implement GitHub Actions Output Feature                                   

pr_agent/algo/utils.py

  • Added github_output function to write review data to GitHub Actions
    output.
  • Imported os module for environment variable access.
  • +10/-0   
    pr_reviewer.py
    Use GitHub Output in PR Reviewer                                                 

    pr_agent/tools/pr_reviewer.py

  • Utilized github_output to write review data to GitHub Actions output.
  • +2/-1     
    Configuration changes
    github_action_runner.py
    Configure GitHub Actions Output Setting                                   

    pr_agent/servers/github_action_runner.py

  • Added logic to read GITHUB_ACTION_CONFIG.ENABLE_OUTPUT from
    environment and set it in settings.
  • +2/-0     
    configuration.toml
    Add Configuration for GitHub Actions Output                           

    pr_agent/settings/configuration.toml

    • Added enable_output configuration under github_action_config.
    +1/-0     
    Tests
    test_github_output.py
    Test GitHub Actions Output Functionality                                 

    tests/unittest/test_github_output.py

  • Added tests for github_output function with enabled, disabled, and
    unset output scenarios.
  • +40/-0   

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @codiumai-pr-agent-pro codiumai-pr-agent-pro bot added the enhancement New feature or request label Apr 9, 2024
    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Description updated to latest commit (9d45e39)

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are straightforward and involve adding a new feature with a clear purpose. The complexity is low, and the changes are well-contained within specific areas of the codebase.

    🏅 Score

    85

    🧪 Relevant tests

    Yes

    🔍 Possible issues

    File Handling: The github_output function opens a file for writing without checking if the file already exists or if the path is valid. This could potentially overwrite existing files or lead to exceptions if the path is not writable.

    Environment Variable Dependency: The feature relies on the GITHUB_OUTPUT environment variable being set. If this variable is not set or is misconfigured, the feature will not work as expected.

    🔒 Security concerns

    No

    🔀 Multiple PR themes

    No


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Add a check for the GITHUB_OUTPUT environment variable before writing to it.

    Consider checking if the GITHUB_OUTPUT environment variable is set before attempting to
    write to it. This can prevent potential errors when the environment variable is not set.
    You can raise an exception or log a warning if the variable is not found.

    pr_agent/algo/utils.py [670-673]

     output = os.getenv('GITHUB_OUTPUT')
    +if not output:
    +    raise ValueError("GITHUB_OUTPUT environment variable is not set.")
     key_data = output_data.get(key_name, {})
     with open(output, 'w') as f:
         f.write(f"{key_name}={json.dumps(key_data, indent=None, ensure_ascii=False)}")
     
    Bug
    Convert the ENABLE_OUTPUT setting string to a boolean explicitly.

    The get_setting_or_env function call for GITHUB_ACTION_CONFIG.ENABLE_OUTPUT should
    explicitly convert the environment variable string to a boolean. This ensures that the
    setting is correctly interpreted as a boolean value.

    pr_agent/servers/github_action_runner.py [62-63]

    -enable_output = get_setting_or_env("GITHUB_ACTION_CONFIG.ENABLE_OUTPUT", False)
    +enable_output = get_setting_or_env("GITHUB_ACTION_CONFIG.ENABLE_OUTPUT", "False").lower() in ('true', '1', 't')
     get_settings().set("GITHUB_ACTION_CONFIG.ENABLE_OUTPUT", enable_output)
     
    Maintainability
    Use consistent file paths in tests for GITHUB_OUTPUT.

    It's recommended to use tmp_path / 'output' consistently instead of switching to tmp_path
    / 'output.json' in the test for test_github_output_enabled. This ensures that the test
    accurately reflects the intended use of the GITHUB_OUTPUT environment variable.

    tests/unittest/test_github_output.py [8-14]

     monkeypatch.setenv('GITHUB_OUTPUT', str(tmp_path / 'output'))
     ...
    -with open(str(tmp_path / 'output.json'), 'r') as f:
    +with open(str(tmp_path / 'output'), 'r') as f:
     
    Performance
    Check if key_data is empty before writing to the file.

    To improve the performance and reduce the risk of file handling errors, it's advisable to
    check if key_data is empty before opening and writing to the file. This avoids unnecessary
    file operations.

    pr_agent/algo/utils.py [671-673]

     key_data = output_data.get(key_name, {})
    -with open(output, 'w') as f:
    -    f.write(f"{key_name}={json.dumps(key_data, indent=None, ensure_ascii=False)}")
    +if key_data:
    +    with open(output, 'w') as f:
    +        f.write(f"{key_name}={json.dumps(key_data, indent=None, ensure_ascii=False)}")
     
    Best practice
    Add error handling for file operations.

    For better error handling and debugging, consider logging an error or warning if the
    output file cannot be opened or written to, instead of silently failing.

    pr_agent/algo/utils.py [672-673]

    -with open(output, 'w') as f:
    -    f.write(f"{key_name}={json.dumps(key_data, indent=None, ensure_ascii=False)}")
    +try:
    +    with open(output, 'w') as f:
    +        f.write(f"{key_name}={json.dumps(key_data, indent=None, ensure_ascii=False)}")
    +except IOError as e:
    +    logging.error(f"Failed to write to {output}: {e}")
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @idubnori
    Copy link
    Contributor Author

    idubnori commented Apr 9, 2024

    Convert to draft.

    TODO

    • change to default is true if use in github actions.
    • test: config is not existed case (e.g. use in other than gh actions)

    @idubnori idubnori marked this pull request as draft April 9, 2024 16:25
    @idubnori idubnori marked this pull request as ready for review April 10, 2024 00:26
    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Description updated to latest commit (3412095)

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are straightforward and well-scoped, involving updates to configuration handling, utility function additions, and their usage in the main logic. The addition of tests for the new functionality is a good practice that helps in understanding the impact and correctness of the changes.

    🏅 Score

    85

    🧪 Relevant tests

    Yes

    🔍 Possible issues

    Error Handling: The github_output function does not handle errors that might occur while writing to the file. This could lead to uncaught exceptions if, for example, the file path specified in GITHUB_OUTPUT does not exist or is not writable.

    Environment Variable Dependency: The implementation assumes the GITHUB_OUTPUT environment variable is always correctly set when GITHUB_ACTION_CONFIG.ENABLE_OUTPUT is True. There might be scenarios where this variable is not set, leading to potential failures.

    🔒 Security concerns

    No

    🔀 Multiple PR themes

    No


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Add a check to ensure GITHUB_OUTPUT environment variable is set before writing to it.

    Consider checking if the GITHUB_OUTPUT environment variable is set before attempting to
    write to it. This can prevent potential errors when the environment variable is not set.

    pr_agent/algo/utils.py [670-673]

     output = os.getenv('GITHUB_OUTPUT')
    -key_data = output_data.get(key_name, {})
    -with open(output, 'w') as f:
    -    f.write(f"{key_name}={json.dumps(key_data, indent=None, ensure_ascii=False)}")
    +if output:
    +    key_data = output_data.get(key_name, {})
    +    with open(output, 'w') as f:
    +        f.write(f"{key_name}={json.dumps(key_data, indent=None, ensure_ascii=False)}")
    +else:
    +    # Handle the case where GITHUB_OUTPUT is not set
     
    Convert environment variable to boolean explicitly when setting GITHUB_ACTION_CONFIG.ENABLE_OUTPUT.

    For consistency and to avoid potential bugs, consider explicitly converting the
    environment variable value to a boolean when setting GITHUB_ACTION_CONFIG.ENABLE_OUTPUT.

    pr_agent/servers/github_action_runner.py [62-63]

    -enable_output = get_setting_or_env("GITHUB_ACTION_CONFIG.ENABLE_OUTPUT", True)
    +enable_output = get_setting_or_env("GITHUB_ACTION_CONFIG.ENABLE_OUTPUT", "True").lower() in ('true', '1', 't')
     get_settings().set("GITHUB_ACTION_CONFIG.ENABLE_OUTPUT", enable_output)
     
    Best practice
    Use a context manager for temporary changes to GITHUB_ACTION_CONFIG.ENABLE_OUTPUT.

    To improve code maintainability and readability, consider using a context manager for
    setting and restoring the original state of GITHUB_ACTION_CONFIG.ENABLE_OUTPUT.

    pr_agent/algo/utils.py [667-668]

    -if get_settings().github_action_config.enable_output is False:
    -    return
    +@contextlib.contextmanager
    +def github_output_enabled():
    +    original_state = get_settings().github_action_config.enable_output
    +    try:
    +        yield
    +    finally:
    +        get_settings().github_action_config.enable_output = original_state
     
    +with github_output_enabled():
    +    # Your code that requires GITHUB_ACTION_CONFIG.ENABLE_OUTPUT to be temporarily enabled/disabled
    +
    Add exception handling around the call to github_output.

    To ensure the robustness of the code, consider handling potential exceptions that may
    occur when calling github_output, such as issues with file writing.

    pr_agent/tools/pr_reviewer.py [195]

    -github_output(data, 'review')
    +try:
    +    github_output(data, 'review')
    +except Exception as e:
    +    # Handle the exception, e.g., log it or notify the user
    +    print(f"Failed to write GitHub output: {e}")
     
    Use a pytest fixture for GITHUB_OUTPUT environment setup in tests.

    To improve test isolation and reliability, consider using a fixture to manage the
    environment setup and teardown for GITHUB_OUTPUT.

    tests/unittest/test_github_output.py [6-12]

    -monkeypatch.setenv('GITHUB_OUTPUT', str(tmp_path / 'output'))
    -output_data = {'key1': {'value1': 1, 'value2': 2}}
    -key_name = 'key1'
    +@pytest.fixture
    +def github_output_env(tmp_path, monkeypatch):
    +    monkeypatch.setenv('GITHUB_OUTPUT', str(tmp_path / 'output'))
    +    yield str(tmp_path / 'output')
    +    # Teardown code here if necessary
    +
    +def test_github_output_enabled(github_output_env):
    +    output_data = {'key1': {'value1': 1, 'value2': 2}}
    +    key_name = 'key1'
         
    -github_output(output_data, key_name)
    +    github_output(output_data, key_name)
    +    # Continue with the test using github_output_env
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    Comment on lines 666 to 673
    def github_output(output_data: dict, key_name: str):
    if get_settings().github_action_config.enable_output is False:
    return

    output = os.getenv('GITHUB_OUTPUT')
    key_data = output_data.get(key_name, {})
    with open(output, 'w') as f:
    f.write(f"{key_name}={json.dumps(key_data, indent=None, ensure_ascii=False)}")
    Copy link
    Collaborator

    Choose a reason for hiding this comment

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

    (1)
    change name to 'github_action_output'
    it can be confusing to call it as a generic 'github_output' name

    (2) wrap everything with try-except.
    see also PR-Agent feedback:
    "
    Environment Variable Dependency: The feature relies on the GITHUB_OUTPUT environment variable being set.
    "
    Will it always be set ?

    (3)
    change
    " if get_settings().github_action_config.enable_output is False:
    return"

    to getter only:
    if get_settings().get("github_action_config.enable_output", False)

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    @mrT23 Thank you for your reivewing.

    • (1) : fixed.
    • (2) : fixed, refined & add test code.
      I think GITHUB_OUTPUT env is always available on github actions runner. Refined the output implementation based on most using codebases. (ref.1 / ref.2)
      Note that this feature is implicitly disabled in the configuration if use in other than github actions. so, I think no side-effect.
    • (3): fixed.

    @mrT23
    Copy link
    Collaborator

    mrT23 commented Apr 10, 2024

    Hi @idubnori , thanks for the PR.
    i gave some minor notes.

    Please also update the documentation:
    https://github.com/Codium-ai/pr-agent/blob/main/docs/docs/usage-guide/automations_and_usage.md#github-action
    showing how to turn on this feature, and what it will be its impact

    @idubnori
    Copy link
    Contributor Author

    Updated the doc. Please confirm it and let me know if you have any suggestions. thanks.

    @mrT23 mrT23 merged commit af8ca7d into Codium-ai:main Apr 11, 2024
    1 check passed
    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