[DNM] Feast changes by Vikash082 · Pull Request #2 · oneconvergence/feast

Vikash082 · 2022-07-13T02:05:47Z

No description provided.

- changes to proxy materialize call to remote online server. - refactored online store with provision of driver.

- Added .env file to set Online store config.

(cherry picked from commit a7f2630)

- The module name of provider, offline store & online store is changed. - The .env file is now expected to be in HOME directory with all the config details.

- in materialize api, client now send project name also.

Other changes: - added "Host" in request header for online server - instanceof() fix - changed DKUBE_URL

dockerfile and support script to prepare feast ds image

… into feast_changes

songole · 2022-07-13T23:40:11Z

+
+
+RUN cd /opt \
+    && git clone https://github.com/oneconvergence/feast.git \


Just a nit: Would --recurse-submodules clone everything in one go.

songole · 2022-07-13T23:42:18Z

+    apt-get -y autoremove && \
+    rm -rf /var/lib/apt/lists/*
+
+COPY install_python.sh install_python.sh


is this script not in the repo?

songole · 2022-07-13T23:56:03Z

+    def __init__(self, **kwargs) -> None:
+        self.dkube_ip = kwargs.get("dkube_ip", "192.168.x.y")
+        self.dkube_port = kwargs.get("dkube_port", 32222)
+        self.dkube_endpoint = kwargs.get("dkube_endpoint", True)


Can we just take dkube_url and token? Can we also get from env right here as opposed to in some caller function?

songole · 2022-07-13T23:57:11Z

+
+class DkubeProvider(PassthroughProvider):
+    """
+    (VK): We need to see if we can work with the PassthroughProvider


Is the comment still valid?

songole · 2022-07-14T00:25:00Z

+                verify=verify,
+                params=params,
+                timeout=timeout)
+        return self.process_response(resp)


If this is file is mostly independent of the rest of feast, can it be moved to dkube repo?

songole · 2022-07-14T00:28:00Z

+    def teardown(self, **kwargs):
+        self.dkube.delete_registry(kwargs['project'])
+
+    def validate_infra_update_with_registry(self, update_infra):


What is infra_update call used for?

This is called before commiting the changes to registry as part of apply call.

songole · 2022-07-14T01:09:15Z

+        self,
+        config: RepoConfig,
+        tables_to_delete: Sequence[Union[FeatureTable, FeatureView]],
+        tables_to_keep: Sequence[Union[FeatureTable, FeatureView]],


You mean update as opposed to keep?

If a table is updated, does it appear in both delete and keep?

If there is new FV definition, the older one with registry will get deleted and the new definition will get stored. If the table is updated it will be part of table_to_keep.

songole · 2022-07-14T01:10:57Z

+        # Replace the code below in order to define your own custom teardown
+        # operations
+        self.initialize(config)
+        self.driver["remote"].teardown(config, tables, entities)


Are we not tearing down all the project related resources? If so, we just need project?

This is online store teardown. This is not entire project teardown.

Is Online store not at project level? can we not figure out the rest if we just know project?
Or, is that how they defined the API?

This is how the APIs are defined. We need to extend this and take action based on that.

songole · 2022-07-14T01:14:44Z

@@ -0,0 +1,520 @@
+from dataclasses import dataclass


Is this offline source? Why would this be different from what feast supports?

Or, does feast support mysql offline source? If it doesn't, we can propose this to the community?

songole · 2022-07-14T01:27:09Z

+        port = self.dkube_store["port"]
+        user = self.dkube_store["user"]
+        password = self.dkube_store["secret"]
+        database = self.dkube_store["db"]


Basically, this supports an external mysql server (like dbaas) as online server, not the one dkube launches. Is this true?

Yes, this driver is to connect any MYSQL server.

songole · 2022-07-15T00:16:00Z

+         """
+        if "dkuberegistrystore" == self._registry._registry_store.__class__.__name__.lower():
+            # Proxy materialize call to online server.
+            self._get_provider().proxy_materialize_incremental(


proxy_xxx() <-- Are these specific calls we added?

songole · 2022-07-15T00:18:22Z

+         )
+         return job
+
+    def proxy_materialize(


Are we using PassThrough provider?

songole · 2022-07-15T00:47:25Z

+        }
+        for fs in services_to_update]
+
+        infra_update = {


In feast main branch, they have a way to calculate diff. Is it something new? or, just refactored?

https://github.com/feast-dev/feast/tree/master/sdk/python/feast/diff

songole · 2022-07-15T00:52:05Z

+            "user": self.user,
+            "offline_dataset": self.offline_dataset
+        }
+        self.online_server_client.delete("api/v1/teardown",


Can we call dkube sdk whenever we need to make API calls?

This call goes for knative service independent of controller. This is the reason, not part of Dkube SDK.

songole · 2022-07-15T00:55:39Z

+        dclient = get_dkube_client()
+        ods = dclient.get_dataset(USER, offline_ds)
+        return {
+            "user": ods["datum"]["sql"]["username"],


Can we discuss this?

- changes in registry to send the changed resource per object basis. - changes of 'event_timestamp_column' to 'timestamp_field'

- BearerToken tested - feature store fix where feature store is sending duplicate values.

Vikash082 added 27 commits January 21, 2022 11:44

adding feast submodule

19e4d22

added custom provider

1bddb2d

adding patch file

4e6f013

changed patch format

b796f48

Added install instructions in README

2d09995

Changes to delegate online store processing to remote server

99233b7

- changes to proxy materialize call to remote online server. - refactored online store with provision of driver.

Modified online store to load both local & remote driver

30c365e

- Added .env file to set Online store config.

Added requirements.txt for client SDK

56ecc1f

Registry config cleanup

e6e219c

(cherry picked from commit a7f2630)

Minor changes in mysqlserver query

39cf29e

Module renaming of provider

ea716ac

- The module name of provider, offline store & online store is changed. - The .env file is now expected to be in HOME directory with all the config details.

mysqlserver connect_args bug fix

d72078e

Refactored "dkube" folder to "dkubefs"

99f9202

dkube sdk addition

551bd85

utility changes, registry changes, dkube client changes

179e04d

Modified materialize api client to add "project"

fa555fb

- in materialize api, client now send project name also.

Added instructions in README

3962ea2

env variable name changes according to Dkube

5ffe4b1

Other changes: - added "Host" in request header for online server - instanceof() fix - changed DKUBE_URL

removed the redundant arg in mysqlserver driver

75e44c8

mysql 'auth_plugin' changes in connect_args

8d0362c

removed DKUBE_DATASET environment variable

a08395e

Changed the 'mysql-connector' with 'mysql-connector-python'

10aba6e

Added depepndency packages of python3.9 for feast

92c172f

dockerfile and support script to prepare feast ds image

50bdb50

Merge pull request #1 from oneconvergence/feast_changes_dkrfile

e194f85

dockerfile and support script to prepare feast ds image

Changes to support username and offline-dataset in client

5e4c830

Merge branch 'feast_changes' of https://github.com/oneconvergence/feast…

86a54b1

… into feast_changes

Vikash082 requested a review from songole July 13, 2022 02:05

songole reviewed Jul 14, 2022

View reviewed changes

songole reviewed Jul 15, 2022

View reviewed changes

Comment thread feast.patch

)

return job

+

+ def proxy_materialize(

songole Jul 15, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using PassThrough provider?

songole reviewed Jul 15, 2022

View reviewed changes

Vikash082 added 8 commits July 15, 2022 15:42

Removed IP, port as args from dkubeclient

bc7a9d9

Added doc string & updated README

acdb19c

removed python-decouple module

415d440

bumping feast to v0.22-branch

de8728f

Added Git based feast project rendering

22546bc

latest feast patch

93decd1

update dkube project at time of project creation

5a73d58

Changes for git based feast init and other changes

c2b0e57

- changes in registry to send the changed resource per object basis. - changes of 'event_timestamp_column' to 'timestamp_field'

Vikash082 force-pushed the feast_changes branch from a1cf5d3 to c2b0e57 Compare September 15, 2022 08:02

Vikash082 added 4 commits September 19, 2022 14:00

datasource event_timestamp column field fix and minor fixes

e160a53

- BearerToken tested - feature store fix where feature store is sending duplicate values.

changed the provider manipulation of table name

e994241

updated patch

e0c7571

feast sdk image fix

9cf5655

Vikash082 force-pushed the feast_changes branch from 0563f52 to 9cf5655 Compare October 5, 2022 16:33



		RUN cd /opt \
		&& git clone https://github.com/oneconvergence/feast.git \

Uh oh!

Conversation

Vikash082 commented Jul 13, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants