[db-executable-env] Phase 2: DatabaseExecutableEnvironment + tool + connection-config skeleton#2468
Draft
aniruddh-alt wants to merge 3 commits into
Conversation
Abstract base for envs that run user-supplied dotted-path executors. Subclasses provide the per-call execution context (DB conn, HTTP client, FS root, ...) via _build_execution_context. The base owns batch step() dispatch to _step_one, the _absorb_result post-hook, and the close lifecycle. This phase ships the skeleton: class shape, batch step() dispatch, and abstract _build_execution_context hook. Concrete _step_one (executor resolution, ToolResult validation, output_schema validation, _absorb_result invocation) lands in a follow-on phase. ExecutableTool is a small ToolParams subclass enforcing a non-empty executor dotted path. First phase of the db-executable-env trunk chain; targets the trunk branch, will integrate to main with the rest of the feature.
…tion-config skeleton Skeleton for the first concrete ExecutableEnvironment transport. Three new units land here: - DatabaseConnectionConfig (src/oumi/core/configs/params/) — structured-vs-DSN XOR validation, pool / timeout numeric bounds, password-via-env-var hygiene. resolve_url() is deferred to the implementation phase (the SQLAlchemy dependency lands then). - DatabaseExecutableTool (src/oumi/environments/) — full small dataclass. Adds optional statement_timeout_ms; env-side cross-validation against the env-level timeout lives on the env (implementation phase). - DatabaseExecutableEnvironment (src/oumi/environments/) — skeleton class registered as env_type "database". from_params and _build_execution_context raise NotImplementedError until the implementation phase wires the SQLAlchemy engine, dialect guards (Postgres / MySQL / SQLite read-only + statement_timeout), DBAPIError auto-wrap, and audit logging. Branched off Phase 1 so the abstract base imports resolve; will rebase onto the trunk once Phase 1 (#2467) merges.
|
|
4 tasks
Strips "skeleton phase", "arrives in a later phase", and forward-reference phrasing from docstrings and NotImplementedError messages across the executable + database executable skeletons. Also drops the @contextmanager wrapper on the not-yet-implemented _build_execution_context since it raises unconditionally — the return-type annotation alone satisfies the abstract contract. The same edits to executable_environment.py / executable_tool.py / test_executable_environment.py mirror the cleanup landed on Phase 1's branch; rebasing onto trunk later will collapse them to no-ops.
shanghongsim
approved these changes
Jun 2, 2026
|
|
||
|
|
||
| def test_tool_params_cls_is_database_executable_tool(): | ||
| """The env binds to ``DatabaseExecutableTool``.""" |
Contributor
There was a problem hiding this comment.
The phrase "binds to" is a bit confusing here. It reads like the env owns a DatabaseExecutableTool (an instance relationship), when envs actually owns tools (from what I understand from our discussions)
|
|
||
|
|
||
| def test_default_tool_params_cls_is_executable_tool(): | ||
| """ExecutableEnvironment binds to ExecutableTool by default.""" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Phase 2 of the db-executable-env phase chain, target branch:
aniruddh-alt/db-executable-env-trunk.What changed: Adds the skeleton for the first concrete
ExecutableEnvironmenttransport —DatabaseExecutableEnvironment,DatabaseExecutableTool, andDatabaseConnectionConfig. Registersenv_type: databasein the env registry.Why: The abstract base from Phase 1 needs a first consumer to validate the shape. The database transport is the highest-value first target — it exercises the hardest concerns (pooling, dialect differences, error wrapping) and is the basis for the EHR DB example. The full implementation is large; this PR ships only the type surface so reviewers can sign off on the shape before engine wiring lands.
Phase chain context
aniruddh-alt/db-executable-env-trunkWhat's in this PR (456 lines)
src/oumi/core/configs/params/database_connection_params.pyDatabaseConnectionConfig— fields for structured mode (driver / host / port / database / username /password_env_var) and DSN mode (dsn_env_var), mutually exclusive. Pool sizing andconnect_timeout_svalidation.resolve_url()is deferred to the implementation phase (SQLAlchemy dep arrives then).src/oumi/environments/database_executable_tool.pystatement_timeout_ms. Env-side cross-validation against env-level timeout lives on the env (next phase).src/oumi/environments/database_executable_environment.py@register_environment("database"),_executor_context_kwarg = "db",tool_params_cls = DatabaseExecutableTool.from_paramsand_build_execution_contextraiseNotImplementedErroruntil the engine phase.DatabaseExecutableEnvironmentKwargscarriesconnection,read_only,statement_timeout_ms,audit.src/oumi/environments/__init__.pytests/unit/environments/test_database_executable_tool.pystatement_timeout_ms;create()round-trip.tests/unit/environments/test_database_executable_environment.py_executor_context_kwarg == "db";from_paramsraisesNotImplementedError;Kwargsrejects missing connection and non-positive timeout; connection dict coercion works.tests/unit/core/configs/params/test_database_connection_params.pyconnect_timeout_sbounds.Why a skeleton phase
This PR adds no runtime dependencies (no
sqlalchemy). The skeleton declares the type surface and registers the env type so downstream config code can reference it; the actual engine wiring + dialect guards + error wrapping land in the next phase along with the SQLAlchemy dep.Each piece has been pre-validated for issues flagged on #2441:
DatabaseConnectionConfigmode XOR is enforced atfinalize_and_validatetime.Kwargs.statement_timeout_msrejects non-positive values (one of the liberate-bot HIGH-priority findings on feat(environments): add DatabaseExecutableEnvironment #2441 — a per-tool0silently disabling the Postgres timeout). The per-tool side of the same validation lives onDatabaseExecutableTooland the env's_validate_toolsin the implementation phase.Related issues
(No associated issue — direct continuation of the work that lived in PR #2441.)
Before submitting
pytest tests/unit/environments/ tests/unit/core/configs/— 1337 passing on this branch.