Skip to content

GitHub app not installed flow CF-555 #75

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

Merged
merged 2 commits into from
Apr 1, 2025
Merged

Conversation

KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Mar 26, 2025

PR Type

  • Enhancement
  • Tests

Description

  • Integrate Sentry error capture in CF API.

  • Enforce GitHub app check and API key validation.

  • Improve code formatting and type annotations.

  • Refine test formatting and dedent calls.


Changes walkthrough 📝

Relevant files
Enhancement
2 files
cfapi.py
Add Sentry capture and userId key                                               
+4/-2     
cli.py
Enforce API key and GitHub app check                                         
+16/-8   
Formatting
15 files
cmd_init.py
Adjust spacing in GitHub actions installer                             
+1/-1     
console.py
Standardize logger and string quoting                                       
+2/-1     
code_replacer.py
Refine type annotations for preexisting objects                   
+3/-3     
config_parser.py
Improve function signature formatting and spacing               
+4/-2     
function_context.py
Adjust import formatting and cleanup minor code                   
+3/-4     
function_optimizer.py
Reorder import parameters in optimizer module                       
+1/-1     
replay_test.py
Minor formatting update in test replay module                       
+1/-1     
comparator.py
Reformat comparator function and logging calls                     
+1/-1     
equivalence.py
Simplify logging import and code spacing                                 
+1/-2     
parse_test_output.py
Tidy command list and remove extraneous commas                     
+0/-1     
test_runner.py
Format subprocess call commands in test runner                     
+19/-3   
verification_utils.py
Clean up test config and whitespace formatting                     
+1/-3     
end_to_end_test_tracer_replay.py
Append newline and tidy end-to-end tracer test                     
+2/-1     
end_to_end_test_utilities.py
Ensure proper newline at file end in utilities                     
+1/-1     
test_code_replacement.py
Standardize type annotations in replacement tests               
+20/-20 
Refactoring
1 files
code_context_extractor.py
Spread parameters across lines and update helpers               
+74/-41 
Bug fix
1 files
mymodule.py
Remove unwanted BOM character from constant file                 
+1/-1     
Tests
1 files
test_code_context_extractor.py
Append newlines and adjust dedent in context tests             
+5/-1     
Additional files
12 files
pytest_new_process_discovery.py +2/-1     
models.py +19/-10 
test_comparator.py +45/-45 
test_function_dependencies.py +3/-1     
test_get_helper_code.py +6/-6     
test_get_read_only_code.py +20/-6   
test_get_read_writable_code.py +13/-13 
test_get_testgen_code.py +30/-6   
test_instrument_all_and_run.py +3/-1     
test_static_analysis.py +1/-1     
test_unit_test_discovery.py +9/-10   
test_validate_python_code.py +4/-0     

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Sentry Integration

    The new error handling adds Sentry capture along with logging. Please verify that sufficient context is logged and that no sensitive information is exposed during exception capture.

    logger.error(f"Error getting blocklisted functions: {e}")
    sentry_sdk.capture_exception(e)
    return {}
    API Key Enforcement

    The added GitHub API key check and GitHub App validation enforce stricter CI requirements. Please ensure that failure messages are clear and that the logic behaves correctly in various deployment scenarios.

    if env_utils.get_pr_number() is not None:
        assert env_utils.ensure_codeflash_api_key(), (
            "Codeflash API key not found. When running in a Github Actions Context, provide the "
            "'CODEFLASH_API_KEY' environment variable as a secret.\n"
            "You can add a secret by going to your repository's settings page, then clicking 'Secrets' in the left sidebar.\n"
            "Then, click 'New repository secret' and add your api key with the variable name CODEFLASH_API_KEY.\n"
            f"Here's a direct link: {get_github_secrets_page_url()}\n"
            "Exiting..."
        )
    
        repo = git.Repo(search_parent_directories=True)
    
        owner, repo_name = get_repo_owner_and_name(repo)
    
        require_github_app_or_exit(owner, repo_name)
    
    if hasattr(args, "ignore_paths") and args.ignore_paths is not None:
    Test Output Consistency

    The updates using dedented string comparisons in multiple test cases may be sensitive to minor formatting changes. Confirm that these tests remain robust and easy to maintain.

    output = parse_code_and_prune_cst(
        dedent(code), CodeContextType.READ_ONLY, {"TestClass.target_method"}, set(), remove_docstrings=True
    )
    assert dedent(expected).strip() == output.strip()

    Copy link

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @KRRT7 KRRT7 force-pushed the github-app-not-installed-flow branch from 9f05b95 to 3fb7d85 Compare March 26, 2025 01:38
    misrasaurabh1
    misrasaurabh1 previously approved these changes Mar 26, 2025
    @KRRT7 KRRT7 force-pushed the github-app-not-installed-flow branch from 3fb7d85 to 684231a Compare March 31, 2025 18:38
    @KRRT7 KRRT7 requested a review from misrasaurabh1 March 31, 2025 19:36
    misrasaurabh1
    misrasaurabh1 previously approved these changes Mar 31, 2025
    Copy link
    Contributor

    @misrasaurabh1 misrasaurabh1 left a comment

    Choose a reason for hiding this comment

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

    approving, but do fix my one comment

    @KRRT7 KRRT7 force-pushed the github-app-not-installed-flow branch from f3f6fed to 22b32d0 Compare April 1, 2025 00:38
    @KRRT7 KRRT7 force-pushed the github-app-not-installed-flow branch from 22b32d0 to af8a735 Compare April 1, 2025 00:40
    @KRRT7 KRRT7 enabled auto-merge April 1, 2025 00:50
    @KRRT7 KRRT7 merged commit c8950f5 into main Apr 1, 2025
    15 checks passed
    @KRRT7 KRRT7 deleted the github-app-not-installed-flow branch April 1, 2025 05:59
    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