Python SQL Registry

[windoze]

This is the SQL-based Feathr implemented in Python.
The API spec is updated to include some previously missing pieces.
The corresponding UI changes is in another PR #303 submitted by @blrchen

There are still some TODOs:

1. User still needs to manually create the database schema by running the script script/schema.sql.
2. The docker file is included, but there is not ARM template to deploy by one click.

We can talk about these tasks later, right now, please focus on the implementation itself.

[xiaoyongzhu]

I have a few high level comemnts:

1. for folder names etc. we should reflect it's a "SQL" based registry
2. We should add docs so users know how to use this. Should they build docker files? And how do they connect to the docker file? At least I'm not clear on those.
3. Seems like many code are missing comments, which will be hard for other devs to maintain. Please do add comments/descriptions for all the functions.
4. Seems additional clean ups are required.

[xiaoyongzhu]

for those DerivedFeatureDef or AnchorFeatureDef classes, can we rename it so that we know it's specific to registry? Currently it's a bit confusing as there might be duplications on the existing feature definition that is defined here, vs the ones that are available in the "Feathr Core" code.

[windoze]

The Dockerfile is not intent to be used by users directly, it is used by the building process, and we'll setup CI actions to publish ready-to-use docker images to some public repo like DockerHub, @jainr is working on this task.

[xiaoyongzhu]

The Dockerfile is not intent to be used by users directly, it is used by the building process, and we'll setup CI actions to publish ready-to-use docker images to some public repo like DockerHub, @jainr is working on this task.

How would the end users use that pre-built container in this case?

[windoze]

That's why I suggested to split the registry from the feathr client repo.
They share a lot of concepts hence lots of similar but not exactly same classes and types, these classes look similar because they're fundamentally representing the same thing, not same because they're for different purposes, on the client side, they're for user composing and job submitting, on the server side, they're for displaying, storing and indexing.
If we put these to separated components together, the confusion is inevitable.
And I don't think it's not a good idea to re-use client side code, there are too many different logics and processes need to be applied to these entities differently on different side, merging them could lead to many potential bugs and even security issues.

[windoze]

The Dockerfile is not intent to be used by users directly, it is used by the building process, and we'll setup CI actions to publish ready-to-use docker images to some public repo like DockerHub, @jainr is working on this task.

How would the end users use that pre-built container in this case?

Again, this part is covered by the manual that @jainr is currently working on, so let's put the comments about the deploying/running questions in that upcoming user-manual PR.

[xiaoyongzhu]

That's why I suggested to split the registry from the feathr client repo. They share a lot of concepts hence lots of similar but not exactly same classes and types, they're similar because they're representing the same thing, not same because they're for different purposes, on the client side, they're for user composing and job submitting, on the server side, they're for displaying, storing and indexing. If we put these to separated components together, the confusion is inevitable.

Correct me if I'm wrong here, but seems like the registry in this PR is only to be consumed by UI (and the REST API layer). I.e. it's not ready to be consumed by the Python client (say, users cannot call get_features_from_registry() for this registry.

For this specific PR, it should be self contained (that's why you have a dockerfile) and it's equivalent of building another smaller Purview service, I think we should move it under a folder NOT in a separate repo, but rather in the same repo but not in the python client folder. maybe create something under root folder and put it there. Or put it under the ui/ folder.

Also - this folder (https://github.com/linkedin/feathr/tree/main/feathr_project/feathr/api) should be also moved tin a similar way (i.e. not using registry service)

[windoze]

That's why I suggested to split the registry from the feathr client repo. They share a lot of concepts hence lots of similar but not exactly same classes and types, they're similar because they're representing the same thing, not same because they're for different purposes, on the client side, they're for user composing and job submitting, on the server side, they're for displaying, storing and indexing. If we put these to separated components together, the confusion is inevitable.

Correct me if I'm wrong here, but seems like the registry in this PR is only to be consumed by UI (and the REST API layer). I.e. it's not ready to be consumed by the Python client (say, users cannot call get_features_from_registry() for this registry.

For this specific PR, it should be self contained (that's why you have a dockerfile) and it's equivalent of building another smaller Purview service, I think we should move it under a folder NOT in a separate repo, but rather in the same repo but not in the python client folder. maybe create something under root folder and put it there. Or put it under the ui/ folder.

Also - this folder (https://github.com/linkedin/feathr/tree/main/feathr_project/feathr/api) should be also moved tin a similar way (i.e. not using registry service)

Agree.

Please raise an issue and let's make decision on directory structure and do a major restructuring soon, not only for this part, but also other components.

[xiaoyongzhu]

Can we do a few things here?

1. Move this implementation to the project root or somewhere else, rather than under /feathr_project
2. Add a bit on the implementation notes for how this solution works in the README file (like we are only storing edges and nodes, and each nodes have different type, etc.)
3. Add comments in the python files (especially the ones under registry folder) to make it easier to maintain

[xiaoyongzhu]

Can we do a few things here?

1. Move this implementation to the project root or somewhere else, rather than under /feathr_project
2. Add a bit on the implementation notes for how this solution works in the README file (like we are only storing edges and nodes, and each nodes have different type, etc.)
3. Add comments in the python files (especially the ones under registry folder) to make it easier to maintain