diff --git a/python/packages/oracle/README.md b/python/packages/oracle/README.md index 6e64b0c0249..2ee62b0864b 100644 --- a/python/packages/oracle/README.md +++ b/python/packages/oracle/README.md @@ -64,7 +64,7 @@ pass them through `pool_parameters` or use a configured client. Alternatively, pass a configured `oracledb.AsyncConnection` or `oracledb.AsyncConnectionPool` as `client` for advanced authentication. A -supplied client cannot be combined with connection settings and remains +supplied client takes precedence over connection settings and remains caller-owned. A connector-created pool opens on first use and closes on `close()` or async context exit. A collection obtained from a store borrows its client; closing that collection does not close the store. The store must remain diff --git a/python/packages/oracle/agent_framework_oracle/_vector_store.py b/python/packages/oracle/agent_framework_oracle/_vector_store.py index 672baaf577e..dd509e56d56 100644 --- a/python/packages/oracle/agent_framework_oracle/_vector_store.py +++ b/python/packages/oracle/agent_framework_oracle/_vector_store.py @@ -373,8 +373,6 @@ def _create_client( env_file_encoding: str | None, ) -> _Client: if client is not None: - if any(value is not None for value in (dsn, user, password, pool_parameters, env_file_path, env_file_encoding)): - raise ValueError("client cannot be combined with dsn, user, password, pool_parameters, or .env settings.") return _Client(client=client) supplied_pool_parameters = dict(pool_parameters or {}) overrides: dict[str, Any] = { @@ -576,7 +574,8 @@ def __init__( driver but not yet declared by ``OracleSettings``. Named ``dsn``, ``user``, and ``password`` arguments take precedence, followed by this mapping, the selected .env file, process environment, and connector pool defaults. - client: Borrowed async connection or pool in place of connection settings. + client: Borrowed async connection or pool. Takes precedence over connection, + pool, and .env settings and remains caller-owned. env_file_path: Optional selected .env file; no implicit discovery. env_file_encoding: Encoding of the selected .env file. """ @@ -992,7 +991,8 @@ def __init__( driver but not yet declared by ``OracleSettings``. Named ``dsn``, ``user``, and ``password`` arguments take precedence, followed by this mapping, the selected .env file, process environment, and connector pool defaults. - client: Borrowed async connection or pool in place of connection settings. + client: Borrowed async connection or pool. Takes precedence over connection, + pool, and .env settings and remains caller-owned. embedding_generator: Default local embedding client for child collections. env_file_path: Optional selected .env file; no implicit discovery. env_file_encoding: Encoding of the selected .env file. diff --git a/python/packages/oracle/tests/oracle/test_settings.py b/python/packages/oracle/tests/oracle/test_settings.py index 16ed85cad2d..ee59d1610e4 100644 --- a/python/packages/oracle/tests/oracle/test_settings.py +++ b/python/packages/oracle/tests/oracle/test_settings.py @@ -158,12 +158,45 @@ def test_missing_selected_file_is_not_ignored(constructor, tmp_path): constructor(dsn="explicit", user="explicit", password="explicit", env_file_path=str(tmp_path / "missing.env")) -@pytest.mark.parametrize("option", ["dsn", "user", "password", "pool_parameters", "env_file_path", "env_file_encoding"]) -def test_borrowed_client_rejects_connection_options(constructor, option): - client = MagicMock(spec=oracledb.AsyncConnection) - value = {"min": 1} if option == "pool_parameters" else "not-read" - with pytest.raises(ValueError, match="cannot be combined"): - constructor(client=client, **{option: value}) +@pytest.mark.parametrize( + "options", + [ + {"dsn": ""}, + {"user": ""}, + {"password": ""}, + {"pool_parameters": {"min": "invalid", "unsupported_option": True}}, + {"pool_parameters": {}}, + {"env_file_path": "missing.env"}, + {"env_file_encoding": "invalid-encoding"}, + { + "dsn": "ignored", + "user": "ignored", + "password": SecretString("ignored"), + "pool_parameters": {"max": "invalid"}, + "env_file_path": "missing.env", + "env_file_encoding": "invalid-encoding", + }, + ], +) +@pytest.mark.parametrize("client_type", [oracledb.AsyncConnection, oracledb.AsyncConnectionPool]) +async def test_borrowed_client_ignores_connection_options(constructor, monkeypatch, options, client_type): + monkeypatch.setenv("ORACLE_MAX", "not-an-integer") + client = MagicMock(spec=client_type) + client.close = AsyncMock() + with ( + patch.object(module, "load_settings", side_effect=AssertionError("Unexpected settings read")), + patch.object(module.oracledb, "create_pool_async") as create, + ): + instance = constructor(client=client, **options) + assert await instance._client._get_client() is client + assert not instance.managed_client + await instance.close() + await instance.close() + create.assert_not_called() + client.close.assert_not_awaited() + if client_type is oracledb.AsyncConnection: + client.commit.assert_not_called() + client.rollback.assert_not_called() @pytest.mark.parametrize("client_type", [oracledb.AsyncConnection, oracledb.AsyncConnectionPool]) @@ -178,9 +211,10 @@ async def test_borrowed_client_bypasses_settings_and_remains_open(constructor, c client.close.assert_not_awaited() -def test_invalid_client_type_is_rejected(constructor): +@pytest.mark.parametrize("options", [{}, {"dsn": "", "pool_parameters": {"min": "invalid"}}]) +def test_invalid_client_type_is_rejected(constructor, options): with pytest.raises(TypeError, match="client"): - constructor(client=object()) + constructor(client=object(), **options) async def test_store_shares_client_without_rereading_settings(monkeypatch, definition_factory):