Addressed review comments

Author: prateeks99

tests/gsd/test_2000_synthetic_data.py

@@ -13,7 +13,12 @@
 
 import pytest
 import select_ai
-from select_ai import Profile, ProfileAttributes, SyntheticDataAttributes, SyntheticDataParams
+from select_ai import (
+    Profile,
+    ProfileAttributes,
+    SyntheticDataAttributes,
+    SyntheticDataParams,
+)
 
 PROFILE_PREFIX = f"PYSAI_2000_{uuid.uuid4().hex.upper()}"
 
@@ -130,4 +135,3 @@ def test_2008_generate_with_none_attributes(synthetic_profile):
     """Passing None as attributes raises error"""
     with pytest.raises(Exception):
         synthetic_profile.generate_synthetic_data(None)
-

tests/gsd/test_2100_synthetic_data_async.py

@@ -13,7 +13,12 @@
 
 import pytest
 import select_ai
-from select_ai import AsyncProfile, ProfileAttributes, SyntheticDataAttributes, SyntheticDataParams
+from select_ai import (
+    AsyncProfile,
+    ProfileAttributes,
+    SyntheticDataAttributes,
+    SyntheticDataParams,
+)
 
 PROFILE_PREFIX = f"PYSAI_2100_{uuid.uuid4().hex.upper()}"
 
@@ -35,7 +40,9 @@ def async_synthetic_provider(oci_compartment_id):
 
 
 @pytest.fixture(scope="module")
-def async_synthetic_profile_attributes(oci_credential, async_synthetic_provider):
+def async_synthetic_profile_attributes(
+    oci_credential, async_synthetic_provider
+):
     return ProfileAttributes(
         credential_name=oci_credential["credential_name"],
         object_list=[

tests/profiles/test_1400_conversation.py

@@ -16,7 +16,6 @@
 from oracledb import DatabaseError
 from select_ai import Conversation, ConversationAttributes
 
-
 CONVERSATION_PREFIX = f"PYSAI_1400_{uuid.uuid4().hex.upper()}"
 
 
@@ -34,10 +33,7 @@ def _create(**kwargs):
     yield _create
 
     for conv in created:
-        try:
-            conv.delete(force=True)
-        except Exception:
-            pass
+        conv.delete(force=True)
 
 
 @pytest.fixture
@@ -204,4 +200,4 @@ def test_1419_create_with_description_none(conversation_factory):
     )
     attrs = conv.get_attributes()
     assert attrs.title == f"{CONVERSATION_PREFIX}_NONE_DESC"
-    assert attrs.description is None
+    assert attrs.description is None

tests/profiles/test_1500_conversation_async.py

@@ -16,7 +16,6 @@
 from oracledb import DatabaseError
 from select_ai import AsyncConversation, ConversationAttributes
 
-
 CONVERSATION_PREFIX = f"PYSAI_1500_{uuid.uuid4().hex.upper()}"
 
 
@@ -34,15 +33,14 @@ async def _create(**kwargs):
     yield _create
 
     for conversation in created:
-        try:
-            await conversation.delete(force=True)
-        except Exception:
-            pass
+        await conversation.delete(force=True)
 
 
 @pytest.fixture
 async def async_conversation(async_conversation_factory):
-    return await async_conversation_factory(title=f"{CONVERSATION_PREFIX}_ACTIVE")
+    return await async_conversation_factory(
+        title=f"{CONVERSATION_PREFIX}_ACTIVE"
+    )
 
 
 @pytest.mark.anyio
@@ -60,7 +58,9 @@ async def test_1501_create_with_description(async_conversation_factory):
     )
     attributes = await conversation.get_attributes()
     assert attributes.title == f"{CONVERSATION_PREFIX}_HISTORY"
-    assert attributes.description == "LLM's understanding of history of science"
+    assert (
+        attributes.description == "LLM's understanding of history of science"
+    )
 
 
 @pytest.mark.anyio
@@ -110,7 +110,9 @@ async def test_1506_set_attributes_with_none(async_conversation):
 @pytest.mark.anyio
 async def test_1507_delete_conversation(async_conversation_factory):
     """Delete async conversation and validate removal"""
-    conversation = await async_conversation_factory(title=f"{CONVERSATION_PREFIX}_DELETE")
+    conversation = await async_conversation_factory(
+        title=f"{CONVERSATION_PREFIX}_DELETE"
+    )
     await conversation.delete(force=True)
     with pytest.raises(select_ai.errors.ConversationNotFoundError):
         await conversation.get_attributes()
@@ -119,7 +121,9 @@ async def test_1507_delete_conversation(async_conversation_factory):
 @pytest.mark.anyio
 async def test_1508_delete_twice(async_conversation_factory):
     """Deleting an already deleted async conversation raises DatabaseError"""
-    conversation = await async_conversation_factory(title=f"{CONVERSATION_PREFIX}_DELETE_TWICE")
+    conversation = await async_conversation_factory(
+        title=f"{CONVERSATION_PREFIX}_DELETE_TWICE"
+    )
     await conversation.delete(force=True)
     with pytest.raises(DatabaseError):
         await conversation.delete()
@@ -133,7 +137,9 @@ async def test_1509_list_contains_created_conversation(async_conversation):
 
 
 @pytest.mark.anyio
-async def test_1510_multiple_conversations_have_unique_ids(async_conversation_factory):
+async def test_1510_multiple_conversations_have_unique_ids(
+    async_conversation_factory,
+):
     """Multiple async conversations produce unique identifiers"""
     titles = [
         f"{CONVERSATION_PREFIX}_AI",
@@ -165,7 +171,9 @@ async def test_1512_set_attributes_with_invalid_id():
     """Updating async conversation with invalid id raises DatabaseError"""
     conversation = AsyncConversation(conversation_id="fake_id")
     with pytest.raises(DatabaseError):
-        await conversation.set_attributes(ConversationAttributes(title="Invalid"))
+        await conversation.set_attributes(
+            ConversationAttributes(title="Invalid")
+        )
 
 
 @pytest.mark.anyio
@@ -185,9 +193,13 @@ async def test_1514_get_attributes_with_invalid_id():
 
 
 @pytest.mark.anyio
-async def test_1515_get_attributes_for_deleted_conversation(async_conversation_factory):
+async def test_1515_get_attributes_for_deleted_conversation(
+    async_conversation_factory,
+):
     """Fetching attributes after deletion raises ConversationNotFound"""
-    conversation = await async_conversation_factory(title=f"{CONVERSATION_PREFIX}_TO_DELETE")
+    conversation = await async_conversation_factory(
+        title=f"{CONVERSATION_PREFIX}_TO_DELETE"
+    )
     await conversation.delete(force=True)
     with pytest.raises(select_ai.errors.ConversationNotFoundError):
         await conversation.get_attributes()
@@ -196,7 +208,9 @@ async def test_1515_get_attributes_for_deleted_conversation(async_conversation_f
 @pytest.mark.anyio
 async def test_1516_list_contains_new_conversation(async_conversation_factory):
     """List reflects newly created async conversation"""
-    conversation = await async_conversation_factory(title=f"{CONVERSATION_PREFIX}_LIST")
+    conversation = await async_conversation_factory(
+        title=f"{CONVERSATION_PREFIX}_LIST"
+    )
     listed = [item async for item in AsyncConversation.list()]
     assert any(
         item.conversation_id == conversation.conversation_id for item in listed
@@ -211,9 +225,13 @@ async def test_1517_list_returns_async_conversation_instances():
 
 
 @pytest.mark.anyio
-async def test_1518_get_attributes_without_description(async_conversation_factory):
+async def test_1518_get_attributes_without_description(
+    async_conversation_factory,
+):
     """Async conversation created without description has None description"""
-    conversation = await async_conversation_factory(title=f"{CONVERSATION_PREFIX}_NO_DESC")
+    conversation = await async_conversation_factory(
+        title=f"{CONVERSATION_PREFIX}_NO_DESC"
+    )
     attributes = await conversation.get_attributes()
     assert attributes.title == f"{CONVERSATION_PREFIX}_NO_DESC"
     assert attributes.description is None
@@ -228,4 +246,4 @@ async def test_1519_create_with_description_none(async_conversation_factory):
     )
     attributes = await conversation.get_attributes()
     assert attributes.title == f"{CONVERSATION_PREFIX}_NONE_DESC"
-    assert attributes.description is None
+    assert attributes.description is None

tests/profiles/test_1600_generate.py

@@ -9,16 +9,21 @@
 1600 - Profile generate API tests
 """
 
+import json
 import uuid
 
 import oracledb
 import pandas as pd
 import pytest
 import select_ai
-from select_ai import Conversation, ConversationAttributes, Profile, ProfileAttributes
+from select_ai import (
+    Conversation,
+    ConversationAttributes,
+    Profile,
+    ProfileAttributes,
+)
 from select_ai.profile import Action
 
-
 PROFILE_PREFIX = f"PYSAI_1600_{uuid.uuid4().hex.upper()}"
 
 PROMPTS = [
@@ -37,12 +42,12 @@ def generate_provider(oci_compartment_id):
 
 
 @pytest.fixture(scope="module")
-def generate_profile_attributes(oci_credential, generate_provider):
+def generate_profile_attributes(test_env, oci_credential, generate_provider):
     return ProfileAttributes(
         credential_name=oci_credential["credential_name"],
         object_list=[
-            {"owner": "PYTHONUSER", "name": "people"},
-            {"owner": "PYTHONUSER", "name": "gymnast"},
+            {"owner": test_env.test_user, "name": "people"},
+            {"owner": test_env.test_user, "name": "gymnast"},
         ],
         provider=generate_provider,
     )
@@ -61,14 +66,11 @@ def generate_profile(generate_profile_attributes):
         attribute_value="meta.llama-3.1-405b-instruct",
     )
     yield profile
-    try:
-        profile.delete(force=True)
-    except Exception:
-        pass
+    profile.delete(force=True)
 
 
 @pytest.fixture
-def negative_profile(oci_credential, generate_provider):
+def negative_profile(test_env, oci_credential, generate_provider):
     profile_name = f"{PROFILE_PREFIX}_NEG_{uuid.uuid4().hex.upper()}"
     attributes = ProfileAttributes(
         credential_name=oci_credential["credential_name"],
@@ -82,23 +84,31 @@ def negative_profile(oci_credential, generate_provider):
     )
     profile.set_attribute(
         attribute_name="object_list",
-        attribute_value='[{"owner": "PYTHONUSER", "name": "people"},'
-        '{"owner": "PYTHONUSER", "name": "gymnast"}]',
+        attribute_value=json.dumps(
+            [
+                {"owner": test_env.test_user, "name": "people"},
+                {"owner": test_env.test_user, "name": "gymnast"},
+            ]
+        ),
     )
     profile.set_attribute(
         attribute_name="model",
         attribute_value="meta.llama-3.1-405b-instruct",
     )
     yield profile
-    try:
-        profile.delete(force=True)
-    except Exception:
-        pass
+    profile.delete(force=True)
 
 
 def test_1600_action_enum_members():
     """Validate Action enum exposes expected members"""
-    for member in ["RUNSQL", "SHOWSQL", "EXPLAINSQL", "NARRATE", "CHAT", "SHOWPROMPT"]:
+    for member in [
+        "RUNSQL",
+        "SHOWSQL",
+        "EXPLAINSQL",
+        "NARRATE",
+        "CHAT",
+        "SHOWPROMPT",
+    ]:
         assert hasattr(Action, member)
 
 
@@ -167,7 +177,9 @@ def test_1608_narrate(generate_profile):
 def test_1609_chat_session(generate_profile):
     """chat_session provides a session context"""
     conversation = Conversation(attributes=ConversationAttributes())
-    with generate_profile.chat_session(conversation=conversation, delete=True) as session:
+    with generate_profile.chat_session(
+        conversation=conversation, delete=True
+    ) as session:
         assert session is not None
 
 
@@ -194,22 +206,28 @@ def test_1612_generate_showsql(generate_profile):
 
 def test_1613_generate_chat(generate_profile):
     """generate with CHAT returns response"""
-    chat_resp = generate_profile.generate(prompt="Tell me about OCI", action=Action.CHAT)
+    chat_resp = generate_profile.generate(
+        prompt="Tell me about OCI", action=Action.CHAT
+    )
     assert isinstance(chat_resp, str)
     assert len(chat_resp) > 0
 
 
 def test_1614_generate_narrate(generate_profile):
     """generate with NARRATE returns response"""
-    narrate_resp = generate_profile.generate(prompt=PROMPTS[1], action=Action.NARRATE)
+    narrate_resp = generate_profile.generate(
+        prompt=PROMPTS[1], action=Action.NARRATE
+    )
     assert isinstance(narrate_resp, str)
     assert len(narrate_resp) > 0
 
 
 def test_1615_generate_explainsql(generate_profile):
     """generate with EXPLAINSQL returns explanation"""
     for prompt in PROMPTS:
-        explain_sql = generate_profile.generate(prompt=prompt, action=Action.EXPLAINSQL)
+        explain_sql = generate_profile.generate(
+            prompt=prompt, action=Action.EXPLAINSQL
+        )
         assert isinstance(explain_sql, str)
         assert len(explain_sql) > 0
 
@@ -256,8 +274,9 @@ def test_1617_none_prompt_raises_value_error(negative_profile):
 #     """run_sql with non existent table raises DatabaseError"""
 #     negative_profile.set_attribute(
 #         attribute_name="object_list",
-#         attribute_value='[{"owner": "PYTHONUSER", "name": "non_existent_table"}]',
+#         attribute_value=json.dumps(
+#             [{"owner": test_env.test_user, "name": "non_existent_table"}]
+#         ),
 #     )
 #     with pytest.raises(oracledb.DatabaseError):
 #         negative_profile.run_sql(prompt="How many entries in the table")
-