-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Add OPENAI_SESSION_BACKEND environment variable for session backend selection #135
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
Conversation
- Add environment variable OPENAI_SESSION_BACKEND (default: 'openai') - Support 'sqlite' as fallback option for local session storage - Implement create_session() helper function for backend selection - Print session URLs for OpenAI-backed sessions for real-time observation - Update exit/error messages to include session URLs when available - Apply to both interactive and manager-developer modes Co-Authored-By: AJ Steers <[email protected]>
Original prompt from AJ Steers
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Welcome to the Airbyte Connector Builder MCP!Thank you for your contribution! Here are some helpful tips and reminders for your convenience. Testing This Branch via MCPTo test the changes in this specific branch with an MCP client like Claude Desktop, use the following configuration: {
"mcpServers": {
"connector-builder-mcp-dev": {
"command": "uvx",
"args": ["--from", "git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1760135522-openai-session-backend", "connector-builder-mcp"]
}
}
} Testing This Branch via CLIYou can test this version of the MCP Server using the following CLI snippet: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/connector-builder-mcp.git@devin/1760135522-openai-session-backend#egg=airbyte-connector-builder-mcp' --help PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
AI Builder EvaluationsAI builder evaluations run automatically under the following conditions:
A set of standardized evaluations also run on a schedule (Mon/Wed/Fri at midnight UTC) and can be manually triggered via workflow dispatch. Helpful ResourcesIf you have any questions, feel free to ask in the PR comments or join our Slack community. |
📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Run as run.py
participant Env as Env (OPENAI_SESSION_BACKEND)
participant SessionFactory as create_session
participant Session as Session (SQLite/OpenAI)
participant Browser as System Browser
participant Log as Logger
User->>Run: start interactive/manager flow(session_id)
Run->>Env: read OPENAI_SESSION_BACKEND
Run->>SessionFactory: create_session(session_id)
alt backend == "sqlite"
SessionFactory-->>Session: instantiate SQLiteSession(session_id)
else backend == "openai"
SessionFactory-->>Session: instantiate OpenAIConversationsSession()
Session-->>Run: return conversation/session id
Run->>Run: compose session_url
Run->>Log: log session_url
Run->>Browser: attempt to open session_url
else invalid
SessionFactory-->>Run: raise ValueError
Run->>Log: log ValueError and exit
end
Note over Run,Session: main processing loop
alt Normal completion
Run->>Log: log session_url (if any) and trace URL
Run-->>User: exit normally
else KeyboardInterrupt or Error
Run->>Log: log session_url (if any) and trace URL
Run-->>User: exit with error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_agents/src/run.py
(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/run.py (2)
connector_builder_agents/src/tools.py (1)
update_progress_log
(191-215)connector_builder_agents/src/_util.py (1)
open_if_browser_available
(15-26)
🪛 GitHub Actions: Linters
connector_builder_agents/src/run.py
[error] 64-69: Ruff format check failed. 1 file would be reformatted; run 'ruff format' to fix code style issues. Command: 'uv run ruff format --diff .'
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pytest (Fast)
session_url = None | ||
if isinstance(session, OpenAIConversationsSession): | ||
conversation_id = await session._get_session_id() | ||
session_url = f"https://platform.openai.com/conversations/{conversation_id}" | ||
update_progress_log(f"🔗 Session URL: {session_url}", session_state) | ||
open_if_browser_available(session_url) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apply the same resilience when fetching the manager session URL.
Same concern as above: wrap _get_session_id()
so a logging failure doesn’t cancel the entire manager/developer build run.
- if isinstance(session, OpenAIConversationsSession):
- conversation_id = await session._get_session_id()
- session_url = f"https://platform.openai.com/conversations/{conversation_id}"
- update_progress_log(f"🔗 Session URL: {session_url}", session_state)
- open_if_browser_available(session_url)
+ if isinstance(session, OpenAIConversationsSession):
+ try:
+ conversation_id = await session._get_session_id()
+ except Exception as exc:
+ update_progress_log(
+ f"⚠️ Unable to retrieve session URL: {exc}",
+ session_state,
+ )
+ else:
+ session_url = f"https://platform.openai.com/conversations/{conversation_id}"
+ update_progress_log(f"🔗 Session URL: {session_url}", session_state)
+ open_if_browser_available(session_url)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
session_url = None | |
if isinstance(session, OpenAIConversationsSession): | |
conversation_id = await session._get_session_id() | |
session_url = f"https://platform.openai.com/conversations/{conversation_id}" | |
update_progress_log(f"🔗 Session URL: {session_url}", session_state) | |
open_if_browser_available(session_url) | |
session_url = None | |
if isinstance(session, OpenAIConversationsSession): | |
try: | |
conversation_id = await session._get_session_id() | |
except Exception as exc: | |
update_progress_log( | |
f"⚠️ Unable to retrieve session URL: {exc}", | |
session_state, | |
) | |
else: | |
session_url = f"https://platform.openai.com/conversations/{conversation_id}" | |
update_progress_log(f"🔗 Session URL: {session_url}", session_state) | |
open_if_browser_available(session_url) |
/build-connector
❌ Poe command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_agents/src/run.py
(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/run.py (2)
connector_builder_agents/src/tools.py (1)
update_progress_log
(191-215)connector_builder_agents/src/_util.py (1)
open_if_browser_available
(15-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pytest (Fast)
User-reported working URL format uses /logs/conv_{conversation_id} instead of /conversations/{conversation_id}. Updated both interactive and manager-developer modes. Co-Authored-By: AJ Steers <[email protected]>
The conversation_id returned by session._get_session_id() already includes the 'conv_' prefix, so we should not add it again. Fixes URL construction to be: https://platform.openai.com/logs/{conversation_id} instead of: https://platform.openai.com/logs/conv_{conversation_id} Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
connector_builder_agents/src/run.py (3)
50-68
: Ruff formatting failure persists.CI still reports formatting issues in this function. Run
ruff format connector_builder_agents/src/run.py
and commit the changes.Based on past review comment at lines 50-68.
168-174
: Unguarded_get_session_id()
can abort interactive runs.The call to
await session._get_session_id()
is unguarded and can raise exceptions (auth failures, API unavailable, rate limits), preventing the interactive session from starting. Since session URL logging is optional instrumentation, failures should be non-fatal.Apply this diff to wrap the session URL logic in a try/except:
session_url = None if isinstance(session, OpenAIConversationsSession): - conversation_id = await session._get_session_id() - session_url = f"https://platform.openai.com/logs/{conversation_id}" - update_progress_log(f"🔗 Session URL: {session_url}", session_state) - open_if_browser_available(session_url) + try: + conversation_id = await session._get_session_id() + session_url = f"https://platform.openai.com/logs/{conversation_id}" + update_progress_log(f"🔗 Session URL: {session_url}", session_state) + open_if_browser_available(session_url) + except Exception as exc: + update_progress_log( + f"⚠️ Unable to retrieve session URL: {exc}", + session_state, + )Based on past review comments at lines 168-174.
280-286
: Guard session URL lookup in manager/developer flow.Same concern as the interactive mode: the unguarded
await session._get_session_id()
call can raise and prevent the manager/developer build from starting. Session URL retrieval is optional instrumentation and should not block the build.Apply this diff to make the session URL lookup resilient:
session_url = None if isinstance(session, OpenAIConversationsSession): - conversation_id = await session._get_session_id() - session_url = f"https://platform.openai.com/logs/{conversation_id}" - update_progress_log(f"🔗 Session URL: {session_url}", session_state) - open_if_browser_available(session_url) + try: + conversation_id = await session._get_session_id() + session_url = f"https://platform.openai.com/logs/{conversation_id}" + update_progress_log(f"🔗 Session URL: {session_url}", session_state) + open_if_browser_available(session_url) + except Exception as exc: + update_progress_log( + f"⚠️ Unable to retrieve session URL: {exc}", + session_state, + )Based on past review comments at lines 280-286.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_agents/src/run.py
(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/run.py (2)
connector_builder_agents/src/tools.py (1)
update_progress_log
(191-215)connector_builder_agents/src/_util.py (1)
open_if_browser_available
(15-26)
🔇 Additional comments (2)
connector_builder_agents/src/run.py (2)
209-211
: LGTM!The conditional session URL logging in the exit flow is correctly implemented.
218-220
: LGTM!The conditional session URL logging in error and interrupt handlers is correctly implemented across all paths.
Also applies to: 334-336, 340-342
/build-connector
|
OpenAIConversationsSession.__init__ accepts a conversation_id parameter that enables session continuity. Previously the session_id was only passed to SQLiteSession but not to OpenAIConversationsSession, which could break session continuity. Addresses PR feedback: #135 (comment) Co-Authored-By: AJ Steers <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
connector_builder_agents/src/run.py
(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
connector_builder_agents/src/run.py (2)
connector_builder_agents/src/tools.py (1)
update_progress_log
(191-215)connector_builder_agents/src/_util.py (1)
open_if_browser_available
(15-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (6)
connector_builder_agents/src/run.py (6)
4-4
: LGTM: Imports support the new session backend selection.The
os
andOpenAIConversationsSession
imports are necessary for the environment-variable-based backend selection introduced in this PR.Also applies to: 11-11
50-68
: LGTM: Session factory correctly passes session identifiers to both backends.The
create_session()
function properly:
- Reads the environment variable with a sensible default ("openai")
- Passes
session_id
to SQLiteSession andconversation_id
to OpenAIConversationsSession (matching their respective APIs)- Raises a clear ValueError for invalid backend values
The default change from SQLite to OpenAI is a breaking change, but it's documented in the PR objectives.
209-210
: LGTM: Session URLs properly surfaced at exit points.The session URLs are consistently logged alongside trace URLs when users exit the interactive session (both normal exit and KeyboardInterrupt). The conditional checks prevent logging when session_url is None (i.e., when using SQLite backend).
Also applies to: 218-219
280-286
: Consistent session URL handling in manager-developer mode.The session URL retrieval and logging follows the same pattern as the interactive mode, maintaining consistency across both execution paths.
Note: The URL format and private API concerns flagged for lines 168-174 also apply here.
334-335
: LGTM: Session URLs properly surfaced in error and interrupt handlers.The session URLs are consistently logged in both the KeyboardInterrupt handler and the unexpected exception handler, ensuring users can review their sessions even when builds are interrupted or fail. The conditional checks are appropriate.
Also applies to: 340-341
170-170
: Verify use of private session API
No matching public method was found locally; check the externalagents
library docs for a public session/conversation-ID API or document whysession._get_session_id()
is required.
feat: Add OPENAI_SESSION_BACKEND environment variable for session backend selection
Summary
This PR introduces the
OPENAI_SESSION_BACKEND
environment variable to switch between OpenAI Conversations API (default) and SQLite session storage. When using the OpenAI backend, the system now prints session URLs for real-time observation of agent conversations, addressing the need for better online visibility after the previous trace URL approach broke with Arize evals integration.Key Changes:
create_session()
helper function that readsOPENAI_SESSION_BACKEND
env var (defaults to"openai"
)"sqlite"
as fallback option for local session storagehttps://platform.openai.com/conversations/{conversation_id}
OPENAI_SESSION_BACKEND=sqlite
.Review & Testing Checklist for Human
This PR changes default behavior and introduces untested OpenAI integration. Please verify:
https://platform.openai.com/conversations/{conversation_id}
is an educated guess based on API patterns, not verified against actual OpenAI UI. Confirm this URL actually works.OPENAI_SESSION_BACKEND=sqlite
and verify session storage still works as before.await session._get_session_id()
(note the underscore). Verify if there's a public API we should use instead.Recommended Test Plan
connector-builder-mcp
in interactive mode without setting env var (tests OpenAI default)export OPENAI_SESSION_BACKEND=sqlite
(tests SQLite fallback)export OPENAI_SESSION_BACKEND=invalid
(should raise ValueError)Notes
Summary by CodeRabbit
New Features
Improvements