Skip to content

Add support for OpenAI organization in the secrets file#2

Merged
okotek merged 1 commit intomainfrom
feature/support_openai_org
Jul 6, 2023
Merged

Add support for OpenAI organization in the secrets file#2
okotek merged 1 commit intomainfrom
feature/support_openai_org

Conversation

@okotek
Copy link
Copy Markdown
Contributor

@okotek okotek commented Jul 6, 2023

No description provided.

@okotek okotek merged commit 84231f9 into main Jul 6, 2023
@okotek okotek deleted the feature/support_openai_org branch July 6, 2023 07:06
@almog-lv
Copy link
Copy Markdown
Contributor

almog-lv commented Jul 6, 2023

@CodiumAI-Agent

@QodoAI-Agent
Copy link
Copy Markdown

PR Analysis

  • 🎯 Main theme: Adding support for OpenAI organization in the secrets file
  • 🔍 Description and title: No
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No
  • Minimal and focused: Yes, the PR is minimal and focused as it only adds support for OpenAI organization in the secrets file and does not make any other changes.
  • 🔒 Security concerns: No, the PR does not introduce any obvious security concerns as it only adds support for an additional configuration parameter.

PR Feedback

  • 💡 General PR suggestions: The PR lacks a description which is important for understanding the context and purpose of the changes. Please add a meaningful description to the PR. Also, it would be beneficial to add tests to verify the new functionality.

  • 🤖 Code suggestions:

    • relevant file: pr_agent/algo/ai_handler.py
      suggestion content: Consider adding a more specific error message when the OpenAI organization is not found in the settings. This will help in debugging if the key is missing. [important]

    • relevant file: pr_agent/settings/.secrets_template.toml
      suggestion content: Consider adding a comment to explain what the 'org' field represents and where to find it. This will make it easier for users to understand what they need to provide. [medium]

How to use

Tag me in a comment '@CodiumAI-Agent' to ask for a new review after you update the PR.
You can also tag me and ask any question, for example '@CodiumAI-Agent is the PR ready for merge?'

@QodoAI-Agent
Copy link
Copy Markdown

PR Analysis

  • 🎯 Main theme: Adding support for OpenAI organization in the secrets file
  • 🔍 Description and title: No
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No
  • Minimal and focused: Yes, the PR is focused on adding support for OpenAI organization in the secrets file and doesn't contain unrelated changes.
  • 🔒 Security concerns: No, the PR doesn't introduce any obvious security concerns. It's just adding an optional configuration for OpenAI organization.

PR Feedback

  • 💡 General PR suggestions: The PR lacks a description which is crucial for understanding the context and the reason for the changes. It's recommended to always include a meaningful description. Also, it would be beneficial to add tests to verify the new functionality.

  • 🤖 Code suggestions:

    • relevant file: pr_agent/algo/ai_handler.py
      suggestion content: Consider handling the case when the 'org' key is not present in the settings. Currently, if the 'org' key is not present, it might raise an AttributeError. To prevent this, you can use a default value when trying to access the 'org' key. For example: openai.organization = settings.openai.get('org', None) [important]

    • relevant file: pr_agent/settings/.secrets_template.toml
      suggestion content: It would be helpful to add a comment explaining what the 'org' key is used for and how to obtain it, similar to the comment for the 'key'. [medium]

How to use

Tag me in a comment '@CodiumAI-Agent' to ask for a new review after you update the PR.
You can also tag me and ask any question, for example '@CodiumAI-Agent is the PR ready for merge?'

okotek added a commit that referenced this pull request Jul 6, 2023
@okotek okotek restored the feature/support_openai_org branch January 7, 2024 11:50
yochail pushed a commit to yochail/pr-agent that referenced this pull request Feb 11, 2024
…ai_org

Add support for OpenAI organization in the secrets file
yochail pushed a commit to yochail/pr-agent that referenced this pull request Feb 11, 2024
@mrT23
Copy link
Copy Markdown
Contributor

mrT23 commented Jul 16, 2024

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Score
Possible issue
Improve the robustness of the check for the 'OPENAI.ORG' setting

Consider using a more explicit check for the presence of the 'OPENAI.ORG' setting.
The current check might not behave as expected if the setting exists but is set to a
falsy value (like an empty string or None). A more robust approach would be to check
if the key exists in the settings and is not None.

pr_agent/algo/ai_handler.py [15-16]

-if settings.get("OPENAI.ORG", None):
+if "OPENAI.ORG" in settings and settings.openai.org is not None:
     openai.organization = settings.openai.org
 
Suggestion importance[1-10]: 9

Why: The suggestion improves the robustness of the code by ensuring that the 'OPENAI.ORG' setting is both present and not None, which prevents potential issues with falsy values.

9
Possible bug
Add error handling for missing 'OpenAI organization' setting

It's recommended to handle the potential AttributeError that could be raised if
settings.openai.org is not set, similar to how the absence of settings.openai.key is
handled. This will prevent the application from crashing due to a missing
configuration.

pr_agent/algo/ai_handler.py [15-16]

 if settings.get("OPENAI.ORG", None):
-    openai.organization = settings.openai.org
+    try:
+        openai.organization = settings.openai.org
+    except AttributeError:
+        raise ValueError("OpenAI organization is required") from None
 
Suggestion importance[1-10]: 8

Why: This suggestion adds valuable error handling for the 'OpenAI organization' setting, similar to the existing handling for the API key, which enhances the code's robustness.

8
Enhancement
Enhance the comment for the 'org' field to provide clearer usage instructions

To avoid confusion and potential errors, consider providing more guidance on when
and how to use the 'org' field in the comments. This could include instructions on
when it's necessary to specify an organization and the implications of doing so.

pr_agent/settings/.secrets_template.toml [11]

-org = "<ORGANIZATION>"  # Optional, may be commented out.
+org = "<ORGANIZATION>"  # Optional. Specify if your API usage is linked to a specific OpenAI organization. Leave commented if not applicable.
 
Suggestion importance[1-10]: 7

Why: The suggestion improves the clarity of the comments, helping users understand when and how to use the 'org' field, which can prevent potential misconfigurations.

7
Best practice
Add logging for successful configuration of OpenAI settings

To ensure that the settings are correctly loaded and used, consider adding a logging
statement that confirms the successful configuration of the OpenAI API key and
organization. This will help in debugging and verifying that the settings are
applied as expected.

pr_agent/algo/ai_handler.py [14-16]

 openai.api_key = settings.openai.key
 if settings.get("OPENAI.ORG", None):
     openai.organization = settings.openai.org
+logging.info("OpenAI API configured with key: %s and organization: %s", settings.openai.key, settings.openai.org)
 
Suggestion importance[1-10]: 6

Why: Adding logging for successful configuration is a good practice for debugging and verification, although it is not crucial for the functionality of the code.

6

mrT23 pushed a commit that referenced this pull request May 25, 2025
feat: add add_diagram flag in configuration.toml
Tyler-Rak referenced this pull request in Tyler-Rak/pr-agent Nov 12, 2025
- Add new /webhook-parallel endpoint for concurrent PR processing
- Extract process_webhook() function for shared logic
- Add configuration options: enable_parallel_reviews, max_concurrent_reviews
- Implement asyncio.create_task() for true concurrent execution
- Add semaphore-based concurrency limiting (default: 5 concurrent reviews)
- Keep original /webhook endpoint unchanged for backward compatibility
- Add comprehensive architecture documentation with sequence diagrams

Benefits:
- PR #2 latency: 80s -> 41s (49% improvement)
- PR #3 latency: 120s -> 42s (65% improvement)
- 3x throughput improvement

Feature can be enabled by:
1. Setting bitbucket_app.enable_parallel_reviews=true
2. Changing webhook URL to /webhook-parallel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants