[FSTORE-1185] Real-time retrieval of feature vector via RonDB REST API #1228

vatj · 2024-02-19T11:04:25Z

This PR adds/fixes/changes...

please summarize your changes to the code
and make sure to include all changes to user-facing APIs

JIRA Issue: -

Priority for Review: -

Related PRs: -

How Has This Been Tested?

Unit Tests
Integration Tests
Manual Tests on VM

Checklist For The Assigned Reviewer:

- [ ] Checked if merge conflicts with master exist
- [ ] Checked if stylechecks for Java and Python pass
- [ ] Checked if all docstrings were added and/or updated appropriately
- [ ] Ran spellcheck on docstring
- [ ] Checked if guides & concepts need to be updated
- [ ] Checked if naming conventions for parameters and variables were followed
- [ ] Checked if private methods are properly declared and used
- [ ] Checked if hard-to-understand areas of code are commented
- [ ] Checked if tests are effective
- [ ] Built and deployed changes on dev VM and tested manually
- [x] (Checked if all type annotations were added and/or updated appropriately)

…server

python/hsfs/feature_view.py

python/hsfs/core/vector_server.py

python/hsfs/client/rondb_rest_client.py

python/hsfs/core/rondb_engine.py

kennethmhc · 2024-02-20T15:47:39Z

python/hsfs/core/vector_server.py

+                entry=entry,
+                passed_features=passed_features,
+                return_type=self._rondb_engine.RETURN_TYPE_FEATURE_VECTOR,
+            )


a note for future improvement: since the result returned from rest API is a vector, it can be used directly if _apply_transformation use the feature index instead of feature name for transformation. So it does not need to convert from list to dict and then back to list.

That's a good point, I will add it to the list of possible future improvements in the design log

python/hsfs/core/rondb_engine.py

… client in get_feature_vector(s)

kennethmhc

We should discuss with others about if we should support both client in a single fv.

kennethmhc · 2024-02-27T13:02:06Z

python/hsfs/client/external.py

@@ -295,6 +295,9 @@ def replace_public_host(self, url):
        """no need to replace as we are already in external client"""
        return url

+    def _is_external(self):


why set private is it is used by other class?

kennethmhc · 2024-02-27T13:06:31Z

python/hsfs/client/online_store_rest_client.py

+    else:
+        warn(
+            "Online Store Rest Client is already initialised. To reset connection or/and override configuration, "
+            + "use reset_online_store_rest_client or get_instance methods with optional configuration"


get_instance below do not take any arguments

kennethmhc · 2024-02-27T13:10:48Z

python/hsfs/feature_view.py

@@ -78,7 +78,7 @@ def __init__(
        self._query = query
        self._featurestore_id = featurestore_id
        self._feature_store_id = featurestore_id  # for consistency with feature group
-        self._feature_store_name = featurestore_name
+        self._feature_store_name = util.strip_feature_store_suffix(featurestore_name)


this can be a breaking change. maybe create another variable instead?

kennethmhc · 2024-02-27T16:51:57Z

python/hsfs/feature_view.py

@@ -211,9 +215,19 @@ def init_serving(
        training_dataset_version: Optional[int] = None,
        external: Optional[bool] = None,
        options: Optional[dict] = None,
+        init_online_store_sql_client: Optional[bool] = None,


What is the use case of supporting both client in a single feature view instance? It seems that the motivation of using rest client is that sql client is not available. Alternatively, users can create 2 fv instance and init_serving with different clients.

SirOibaf · 2024-03-10T23:04:17Z

python/hsfs/feature_view.py

            options: Additional options as key/value pairs for configuring online serving engine.
                * key: kwargs of SqlAlchemy engine creation (See: https://docs.sqlalchemy.org/en/20/core/engines.html#sqlalchemy.create_engine).
                  For example: `{"pool_size": 10}`
+                * key: "config_online_store_rest_client" - dict, optional. Optional configuration options to override defaults for the Online Store REST Client.
+                * key: "reset_online_store_rest_client" - bool, optional. If set to True, the Online Store REST Client will be reset. Provide


I don't understand what's the purpose of this reset_online_store_rest_client flag?

Force overwriting the connection with new config details

SirOibaf · 2024-03-10T23:08:56Z

python/hsfs/feature_view.py

            options: Additional options as key/value pairs for configuring online serving engine.
                * key: kwargs of SqlAlchemy engine creation (See: https://docs.sqlalchemy.org/en/20/core/engines.html#sqlalchemy.create_engine).
                  For example: `{"pool_size": 10}`
+                * key: "config_online_store_rest_client" - dict, optional. Optional configuration options to override defaults for the Online Store REST Client.


I'm missing which options users can provide in this dictionary.

python/tests/core/test_online_store_rest_client_engine.py

python/tests/core/test_vector_server.py

SirOibaf · 2024-03-10T23:11:49Z

bigquery.json

@@ -0,0 +1 @@
+{"type": "service_account", "project_id": "test"}


This file is generated by some storage connector test. We should put it in the gitignore.

vatj added 21 commits February 11, 2024 11:23

Draft client and api

5ad862c

buiding payload logic for raw feature vector retrieval

bb96a5e

Tweak rondb base client

b0f9006

rest client boilerplate adjusments

aa51e4f

rest client boilerplate cleanup

f7a99e1

Fix missing auth header

4f4a64b

Working prototype

4860aa1

Conversion logic from raw rdrs response to dict feature vector(s)

30fd9ef

Draft support for vecter_server.get_feature_vector(s) via rondb rest …

7c13660

…server

Integration with feature_view API

02622c5

Transform to datetime to match with sql version

bef5a83

Document and handle error in rondb_rest_api class

d38ec2d

Add documentation and some validation to rondb engine

d4929c2

Fix vector server to adapt to rondb_engine change

eb018ff

Skip null return values

cafec65

Merge branch 'master' into FSTORE-1185

dbc862c

Start adding fixtures

7a94497

More progress on unit tests

7bfd356

More unit tests

eb1a0f2

Start with rondb_rest_api unit test and fixtures

c3e50e6

Wrap up unit test on rondb_rest_api

102e8da

vatj requested a review from kennethmhc February 19, 2024 11:04

kennethmhc reviewed Feb 20, 2024

View reviewed changes

vatj added 7 commits February 23, 2024 20:13

Unit tests for rondb client with successful mocking

2d65e36

Merge branch 'master' into FSTORE-1185

e0fc20c

A vector server with initialised rest client will default to the rest…

6cda2cf

… client in get_feature_vector(s)

Init sql or http. Allow default to be based on initialised + override

6ee02a5

Split init and reset connection for rondb client

9344428

Public api for get_feature_vector modified

ce770a8

Add rest options to batch vector client

ea71e64

vatj added 7 commits February 26, 2024 00:00

Add vector_server init unit test

55c0443

Add default client option/memory

214aafb

Prepare for testing

8640cc1

Hide RonDB

0bd9c10

Minor fix doc and test

8a9d40f

Cleanup which

7972716

Merge branch 'master' into FSTORE-1185

4392eeb

vatj force-pushed the FSTORE-1185 branch from e5bdf62 to 4392eeb Compare February 26, 2024 15:19

vatj added 8 commits February 27, 2024 07:32

Minor fix from review comments

855cf05

Fix and unskip tests

93cb4be

Fix mkdocs

98f19bf

More typing errors

ecfd41e

Fix missing feature store name on feature_view create

536a597

Fix feature_store_name inconsistency with backend response

11742ae

Temp fix to allow RestAPIError to work on rdrs error message

2ec4121

Add datetime deserialization for passed_features

a55d68b

vatj force-pushed the FSTORE-1185 branch from 35f4e9e to a55d68b Compare February 27, 2024 14:55

kennethmhc requested changes Feb 27, 2024

View reviewed changes

vatj added 7 commits February 28, 2024 11:00

Logger to rest_client_engine

1b5916b

Merge branch 'master' into FSTORE-1185

4d63b34

Merge branch 'master' into FSTORE-1185

83e57f5

Online retrieval test notebooks mostly passing

b3a2886

Fix value error if timestamp is None

363a564

Fix build_payload

0f4d44f

Allow for Error status by returning a vector filled with None values

0d273fb

SirOibaf reviewed Mar 10, 2024

View reviewed changes

SirOibaf requested changes Mar 10, 2024

View reviewed changes

vatj closed this Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FSTORE-1185] Real-time retrieval of feature vector via RonDB REST API #1228

[FSTORE-1185] Real-time retrieval of feature vector via RonDB REST API #1228

vatj commented Feb 19, 2024

kennethmhc Feb 20, 2024

vatj Feb 21, 2024

kennethmhc left a comment

kennethmhc Feb 27, 2024

kennethmhc Feb 27, 2024

kennethmhc Feb 27, 2024

kennethmhc Feb 27, 2024

SirOibaf Mar 10, 2024

vatj Mar 11, 2024

SirOibaf Mar 10, 2024

SirOibaf Mar 10, 2024

vatj Mar 11, 2024

		@@ -0,0 +1 @@
		{"type": "service_account", "project_id": "test"}

[FSTORE-1185] Real-time retrieval of feature vector via RonDB REST API #1228

[FSTORE-1185] Real-time retrieval of feature vector via RonDB REST API #1228

Conversation

vatj commented Feb 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kennethmhc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment