Sqlite Changes #472

shreyas-damle · 2024-08-13T05:12:20Z

Items covered:

DB layer to configure storage as file vs db and for db, default value is sqlite as of now.
APIs done so far:
- /discover
- /prompt
entity/topic classifier changes to include confidence score.

Pending items:

UTs

* Adding confidence score to entities and topic responses * Adding labels and fixing UTS * Adding utils * Fixing UT * Remove unused imports * Updating topic classifier --------- Co-authored-by: dristy.cd <[email protected]>

* SQLite basic code structure * Changes after testing * Ruff fixes

* Adding confidence score to entities and topic responses * Adding labels and fixing UTS * Adding utils * Fixing UT * Remove unused imports * Updating topic classifier --------- Co-authored-by: dristy.cd <[email protected]>

* Added DB implementation for /prompt API * Updating prompt service --------- Co-authored-by: dristy.cd <[email protected]>

* Discovery api fixes, Update and datetime converted to isoformat * Storage type code optimization * Optimized the way handler method was called based on storage type. * Changes for prompt api * Changes after testing

* Adding confidence score to entities and topic responses (#460) * Adding confidence score to entities and topic responses * Adding labels and fixing UTS * Adding utils * Fixing UT * Remove unused imports * Updating topic classifier --------- Co-authored-by: dristy.cd <[email protected]> * Added changes for prompt group * resolved linting issue * added changes for confidence score for entity classification * added changes for confidence score * review comment changes * review comment changes * review comment changes --------- Co-authored-by: Dristy Srivastava <[email protected]> Co-authored-by: dristy.cd <[email protected]>

srics

Initial comments

srics · 2024-08-13T20:39:54Z

pebblo/app/enums/enums.py

@@ -30,6 +30,7 @@ class CacheDir(Enum):
        f"http://{config_details.get('daemon', {}).get('host')}:"
        f"{config_details.get('daemon', {}).get('port')}"
    )
+    SQLITE_ENGINE = "sqlite:///{}/pebblo.db"


Make the path/location to this .db file configurable with a proper default location

pebblo/app/config/config.py

pebblo/app/config/config_validation.py

* Adding confidence score to entities and topic responses (#460) * Adding confidence score to entities and topic responses * Adding labels and fixing UTS * Adding utils * Fixing UT * Remove unused imports * Updating topic classifier --------- Co-authored-by: dristy.cd <[email protected]> * Added changes for prompt group * resolved linting issue * conflic check fixes * added changes for adding ip address in entity classifier * added test case for ip address --------- Co-authored-by: Dristy Srivastava <[email protected]> Co-authored-by: dristy.cd <[email protected]>

* Storage config code optimized * Loader doc changes

* Chnages for confidence score in /loader/doc as well as UI API * Fixing UTs --------- Co-authored-by: dristy.cd <[email protected]>

* Changes regarding Local UI APIs * Adding AiUser Table * Adding Dashboard API for retrivals * Incorporated code review comments * Adding new filr for local ui APIs for DB * Updated retrievre app API * Added username in AiRetrievals --------- Co-authored-by: dristy.cd <[email protected]>

* Loader doc changes * Renamed sqlite db file name. * Changed docIds to snippetIds * Add feature to show execution time for functions with database operations * LoadId's added in all sql tables

#482) * Adding confidence score to entities and topic responses (#460) * Adding confidence score to entities and topic responses * Adding labels and fixing UTS * Adding utils * Fixing UT * Remove unused imports * Updating topic classifier --------- Co-authored-by: dristy.cd <[email protected]> * Added changes for prompt group * resolved linting issue * conflic check fixes * added changes for adding ip address in entity classifier * added test case for ip address * added fix for entity classification happening in prompt api for prompt --------- Co-authored-by: Dristy Srivastava <[email protected]> Co-authored-by: dristy.cd <[email protected]>

* Fixed issues found while testing. * Ruff fixes.

* Adding retriever API for local UI for DB Apps * Adding delete API for Retriever DB apps

* Added local-ui utils file

* Local UI - LoaderApp details page UI. * Fixed commit issue by adding flag_modified() method. * Fixed regression on local UI with file storage.

pebblo/app/config/config_validation.py

gr8nishan · 2024-08-22T06:21:05Z

pebblo/app/models/sqltables.py

Can these table_name comes from a ENUM?

pebblo/app/service/discovery/discovery_service.py

pebblo/app/storage/sqlite_db.py

gr8nishan · 2024-08-22T06:35:20Z

pebblo/app/service/prompt/prompt_service.py

+                        AiUser, ai_user_obj.dict()
+                    )
+                    if insert_status:
+                        logger.debug(f"Entry: {entry} in AiUser completed")


Do we need to log "entry"?

@dristysrivastava Can you please check this?

gr8nishan · 2024-08-22T06:39:19Z

pebblo/app/utils/utils.py

+    return datetime.now().isoformat()
+
+
+def timeit(func):


Can timeit function have a condition in the beginning because everytime we will calculate the time for each function but not log it.. So can it only run when the deubg mode is on else it just returns from the first line itself

gr8nishan · 2024-08-22T06:48:06Z

pebblo/app/service/loader/loader_doc_service.py

+            return doc_info
+        except Exception as e:
+            logger.error(
+                f"Get Classifier Response Failed for doc: {doc}, Exception: {e}"


Do we need to log doc. Can we do it with some id etc... ?

pebblo/app/service/loader/loader_doc_service.py

gr8nishan · 2024-08-22T06:52:43Z

pebblo/app/service/loader/snippet/snippet.py

+            else:
+                # If entity or topic does not exist in  app coll
+                restricted_data[data] = {
+                    "count": doc_restricted_data.get(data, 0),


i think we can directly assign 0 in "count" no need for get operation

pebblo/app/service/local_ui/loader_apps.py

gr8nishan · 2024-08-22T07:04:44Z

pebblo/app/service/local_ui/loader_apps.py

+        Aggregating all input, processing the data, and generating the final report
+        """
+        logger.debug("Generating final report")
+        logger.debug(f"LoaderApp: {app_data}")


Do we need to log raw data and app data

gr8nishan · 2024-08-22T07:08:38Z

pebblo/app/service/local_ui/retriever_apps.py

+        :return:  sorted retrievals list
+        """
+        sorted_data = sorted(
+            retrievals, key=lambda item: len(item["retrievals"]), reverse=True


Should we use get len(item["retrievals"]),?

yes, we are using len() and the apply sorted() on top of it.

pebblo/app/service/local_ui/retriever_apps.py

pebblo/app/service/local_ui_service.py

* Updating /prompt/ as well as /delete API * Removed multiple commit statements * Moved response models in db response file * Updating models * Updating model fields --------- Co-authored-by: dristy.cd <[email protected]>

gr8nishan · 2024-08-22T09:38:17Z

pebblo/app/service/discovery/discovery_service.py

+    def _get_app_type_and_class(self):
+        AppClass = None
+        app_type = None
+        load_id = self.data.get("load_id") or None


We can add None in get function itself

gr8nishan · 2024-08-22T09:40:23Z

pebblo/app/service/discovery/discovery_service.py

+    @timeit
+    def process_request(self, data):
+        try:
+            self.db = SQLiteClient()


We are creating SqliteClient in every request. Is it possible to create a db client once and use it across when the pebblo server starts? Maybe a Singleton class or any other option?

* Generate PDF at the end of loder/doc API. * Delete loader app

gr8nishan · 2024-08-22T10:08:53Z

pebblo/app/service/discovery/common.py

+
+        exist, existing_ai_app = db.query(app_class, ai_app)
+        if exist and existing_ai_app:
+            logger.debug(f"Application details exists in {app_class.__tablename__}")


We should add a specific prefix in the logger message like - LoaderApp: as we had done in last release

gr8nishan · 2024-08-22T10:10:16Z

pebblo/app/service/loader/loader_doc_service.py

+            else:
+                raise Exception(result)
+        except Exception as err:
+            message = f"PDF report is not generated. Error: {err}"


If this message is going to the UI should we remove ex ?
Should this be done across all apis ..

gr8nishan · 2024-08-22T10:13:04Z

pebblo/app/service/discovery/discovery_service.py

+        else:
+            # Commit will only happen when everything went well.
+            message = "App Discover Request Processed Successfully"
+            logger.debug(message)


We should add an idetifier in the log messsage
This should be done in all the log messages

Co-authored-by: dristy.cd <[email protected]>

* Made sqlite table creation conditional. * Test cases fixed * Ruff fixes.

dristysrivastava and others added 5 commits August 7, 2024 15:42

Adding confidence score to entities and topic responses (#460)

64956e3

* Adding confidence score to entities and topic responses * Adding labels and fixing UTS * Adding utils * Fixing UT * Remove unused imports * Updating topic classifier --------- Co-authored-by: dristy.cd <[email protected]>

Sqlite implementation initial code (#467)

c0cc455

* SQLite basic code structure * Changes after testing * Ruff fixes

Adding confidence score to entities and topic responses (#460)

e3ffe54

* Adding confidence score to entities and topic responses * Adding labels and fixing UTS * Adding utils * Fixing UT * Remove unused imports * Updating topic classifier --------- Co-authored-by: dristy.cd <[email protected]>

Added DB implementation for /prompt API (#468)

9d596a5

* Added DB implementation for /prompt API * Updating prompt service --------- Co-authored-by: dristy.cd <[email protected]>

Discovery API (#471)

add7a7a

* Discovery api fixes, Update and datetime converted to isoformat * Storage type code optimization * Optimized the way handler method was called based on storage type. * Changes for prompt api * Changes after testing

shreyas-damle requested review from Raj725, srics and dristysrivastava August 13, 2024 06:18

srics reviewed Aug 13, 2024

View reviewed changes

gr8nishan and others added 10 commits August 14, 2024 20:34

Loader Doc API DB Implementation (#478)

55a7d98

* Storage config code optimized * Loader doc changes

confidence score UI (#480)

29e441b

Changes for confidence score in /loader/doc as well as UI API (#470)

dff1c6c

* Chnages for confidence score in /loader/doc as well as UI API * Fixing UTs --------- Co-authored-by: dristy.cd <[email protected]>

Loader doc db changes and fixes (#481)

a8796fb

* Loader doc changes * Renamed sqlite db file name. * Changed docIds to snippetIds * Add feature to show execution time for functions with database operations * LoadId's added in all sql tables

Merge branch 'pebblo-0.1.18' into sqlite-changes

1e7c334

Sqlite Fixes (#483)

e5693dd

* Fixed issues found while testing. * Ruff fixes.

Adding details as well as delete APIs for Retriever DB Apps (#484)

0d79d1d

* Adding retriever API for local UI for DB Apps * Adding delete API for Retriever DB apps

shreyas-damle force-pushed the pebblo-0.1.18 branch from 2506711 to 400f638 Compare August 21, 2024 05:42

Added local-ui utils file (#488)

66cd405

* Added local-ui utils file

shreyas-damle changed the base branch from pebblo-0.1.18 to main August 21, 2024 17:40

shreyas-damle changed the base branch from main to pebblo-0.1.18 August 21, 2024 17:40

shreyas-damle changed the base branch from pebblo-0.1.18 to main August 21, 2024 17:40

Sqlite local UI (#485)

7283538

* Local UI - LoaderApp details page UI. * Fixed commit issue by adding flag_modified() method. * Fixed regression on local UI with file storage.