chore: Update docs for offline and online stores #2946

kevjumba · 2022-07-15T19:32:59Z

Signed-off-by: Kevin Zhang [email protected]

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

codecov-commenter · 2022-07-15T19:42:00Z

Codecov Report

Merging #2946 (5bc74db) into master (aa2a86a) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2946      +/-   ##
==========================================
- Coverage   77.65%   77.61%   -0.05%     
==========================================
  Files         186      186              
  Lines       16354    16354              
==========================================
- Hits        12700    12693       -7     
- Misses       3654     3661       +7

Flag	Coverage Δ
integrationtests	`68.58% <ø> (-0.14%)`	⬇️
unittests	`58.51% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
sdk/python/tests/utils/online_read_write_test.py	`93.54% <0.00%> (-6.46%)`	⬇️
.../integration/online_store/test_online_retrieval.py	`96.84% <0.00%> (-3.16%)`	⬇️
sdk/python/feast/infra/online_stores/datastore.py	`87.96% <0.00%> (-1.39%)`	⬇️
...east/infra/materialization/lambda/lambda_engine.py	`93.75% <0.00%> (+1.04%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa2a86a...5bc74db. Read the comment docs.

docs/how-to-guides/adding-a-new-offline-store.md

adchia · 2022-07-20T15:26:02Z

docs/how-to-guides/adding-a-new-offline-store.md

@@ -30,6 +30,8 @@ There are two methods that deal with reading data from the offline stores`get_hi
 * `pull_latest_from_table_or_query` is invoked when running materialization (using the `feast materialize` or `feast materialize-incremental` commands, or the corresponding `FeatureStore.materialize()` method. This method pull data from the offline store, and the `FeatureStore` class takes care of writing this data into the online store.
 * `get_historical_features` is invoked when reading values from the offline store using the `FeatureStore.get_historical_features()` method. Typically, this method is used to retrieve features when training ML models.
 * `pull_all_from_table_or_query` is a method that pulls all the data from an offline store from a specified start date to a specified end date.


might call out that this method is optional since it is only used to work with SavedDatasets

I think you commented on the wrong method , you mean write_logged_features right?

nope. pull_all_from_table_or_query right now in our logic is only exposed for SavedDatasets afaict

adchia · 2022-07-20T15:27:21Z

docs/how-to-guides/adding-a-new-offline-store.md

@@ -6,9 +6,9 @@ Feast makes adding support for a new offline store (database) easy. Developers c

 In this guide, we will show you how to extend the existing File offline store and use in a feature repo. While we will be implementing a specific store, this guide should be representative for adding support for any new offline store.

-The full working code for this guide can be found at [feast-dev/feast-custom-offline-store-demo](https://github.com/feast-dev/feast-custom-offline-store-demo).
+The full working code for this guide can be found at [feast-dev/feast-custom-offline-store-demo](https://github.com/feast-dev/feast-custom-offline-store-demo) and an example of a custom offline store that was contributed by developers can be found [here](https://github.com/feast-dev/feast/pull/2401).


idk if I would use this as an example. It's a really large PR with also online store + registry store components

Would be better if we created some example PR (similar to what is in this guide)

Hm, I can link the spark pr instead. I think the example guide for the feast custom offline store is fine for high level but I dont' see a clear way of implementing some dummy procedures with. more clarity than the feast custom offline store. Actual implementations to reference would actually then be actual real world pr would help them work out the little kinks in implementation. I agree that the postgres one is a little bit too large but I think the spark one is relevant and useful

Could you convert the dummy example in the guide into an actual PR so they can see?

You can also just point users to the directory that has the offline / online store implementations too

adchia · 2022-07-20T15:28:26Z

docs/how-to-guides/adding-a-new-offline-store.md

@@ -128,6 +130,8 @@ Custom offline stores may need to implement their own instances of the `Retrieva

 The `RetrievalJob` interface exposes two methods - `to_df` and `to_arrow`. The expectation is for the retrieval job to be able to return the rows read from the offline store as a parquet DataFrame, or as an Arrow table respectively.

+Users who want to have their offline store support batch materialization (detailed in this [RFC](https://docs.google.com/document/d/1J7XdwwgQ9dY_uoV9zkRVGQjK9Sy43WISEW6D5V9qzGo/edit#heading=h.9gaqqtox9jg6)) will also need to implement `to_remote_storage` to write distribute the reading and writing of offline store records to a distributed framework. If this functionality is not needed, the RetrievalJob will default to local materialization.


Suggested change

Users who want to have their offline store support batch materialization (detailed in this [RFC](https://docs.google.com/document/d/1J7XdwwgQ9dY_uoV9zkRVGQjK9Sy43WISEW6D5V9qzGo/edit#heading=h.9gaqqtox9jg6)) will also need to implement `to_remote_storage` to write distribute the reading and writing of offline store records to a distributed framework. If this functionality is not needed, the RetrievalJob will default to local materialization.

Users who want to have their offline store support scalable batch materialization for online use cases (detailed in this [RFC](https://docs.google.com/document/d/1J7XdwwgQ9dY_uoV9zkRVGQjK9Sy43WISEW6D5V9qzGo/edit#heading=h.9gaqqtox9jg6)) will also need to implement `to_remote_storage` to write distribute the reading and writing of offline store records to a distributed framework. If this is not implemented, Feast will default to local materialization (pulling all records in memory to materialize).

adchia · 2022-07-20T15:30:19Z

docs/how-to-guides/adding-a-new-offline-store.md

@@ -215,7 +219,7 @@ class CustomFileDataSource(FileSource):

 After implementing these classes, the custom offline store can be used by referencing it in a feature repo's `feature_store.yaml` file, specifically in the `offline_store` field. The value specified should be the fully qualified class name of the OfflineStore.&#x20;

-As long as your OfflineStore class is available in your Python environment, it will be imported by Feast dynamically at runtime.
+As long as your OfflineStore class is available in your Python environment, it will be imported by Feast dynamically at runtime. It is crucial to specify the type as the package that Feast can import.


Might be more clear if you left this as a comment in the example feature_store.yaml

adchia · 2022-07-20T15:31:39Z

docs/how-to-guides/adding-a-new-offline-store.md

@@ -260,23 +264,56 @@ driver_hourly_stats_view = FeatureView(

 ## 6. Testing the OfflineStore class

+### Contrib
+
+Generally, new offline stores should go in the contrib folder and have alpha functionality tags. The contrib folder is designated for community contributions that are not fully maintained by Feast maintainers and may contain potential instability and have API changes. It is recommended to add warnings to users that the offline store functionality is still in alpha development and is not fully stable. In order to be classified as fully stable and be moved to the main offline store folder, the offline store should integrate with the full integration test suite in Feast and pass all of the test cases.


alpha functionality tags is ambiguous imo.

Maybe say our standard is to print messages stating it's alpha status? This is also where a sample simple PR we provide is useful

adchia · 2022-07-20T15:39:25Z

docs/how-to-guides/adding-a-new-offline-store.md

+
+### Documentation
+
+Remember to update the documentation for your offline store. This can be found in `docs/reference/offline-stores/` and `docs/reference/data-sources/`. You should also add a reference in `docs/reference/data-sources/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other offline stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the datasource is in alpha development.


nit: "add documentation" instead of update.

There's also additional work around adding python code docs (i.e. call make build-sphinx)

adchia · 2022-07-20T15:39:46Z

docs/how-to-guides/adding-a-new-offline-store.md

+Remember to update the documentation for your offline store. This can be found in `docs/reference/offline-stores/` and `docs/reference/data-sources/`. You should also add a reference in `docs/reference/data-sources/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other offline stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the datasource is in alpha development.
+
+
+An example of a full pull request for adding a custom offline store can be found [here](https://github.com/feast-dev/feast/pull/2401).


again would not use this as an example because it's too large and hard to parse

addressed above.

docs/how-to-guides/adding-support-for-a-new-online-store.md

adchia · 2022-07-20T15:44:13Z

docs/how-to-guides/adding-support-for-a-new-online-store.md

+
+### Documentation
+
+Remember to update the documentation for your online store. This can be found in `docs/reference/online-stores/`. You should also add a reference in `docs/reference/online-stores/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other online stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the online store is in alpha development.


we might additionally want to cover what the data model is

adchia · 2022-07-20T15:44:22Z

go.sum

@@ -87,6 +87,14 @@ github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.
 github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE=
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
+github.com/feast-dev/gopy v0.4.1-0.20220714205859-591500e3215f h1:tTjEpVu4H/ZGh4wo3WETbA9dutNM6bXMXvyZbb9GLCs=


unintentional?

fixed after rebase.

kevjumba · 2022-07-20T18:46:29Z

@adchia pTal

achals · 2022-07-20T19:32:49Z

docs/how-to-guides/adding-a-new-offline-store.md

@@ -128,6 +151,8 @@ Custom offline stores may need to implement their own instances of the `Retrieva

 The `RetrievalJob` interface exposes two methods - `to_df` and `to_arrow`. The expectation is for the retrieval job to be able to return the rows read from the offline store as a parquet DataFrame, or as an Arrow table respectively.

+Users who want to have their offline store support scalable batch materialization for online use cases (detailed in this [RFC](https://docs.google.com/document/d/1J7XdwwgQ9dY_uoV9zkRVGQjK9Sy43WISEW6D5V9qzGo/edit#heading=h.9gaqqtox9jg6)) will also need to implement `to_remote_storage` to distribute the reading and writing of offline store records to a distributed framework. If this is not implemented, Feast will default to local materialization (pulling all records into memory to materialize).


Suggested change

Users who want to have their offline store support scalable batch materialization for online use cases (detailed in this [RFC](https://docs.google.com/document/d/1J7XdwwgQ9dY_uoV9zkRVGQjK9Sy43WISEW6D5V9qzGo/edit#heading=h.9gaqqtox9jg6)) will also need to implement `to_remote_storage` to distribute the reading and writing of offline store records to a distributed framework. If this is not implemented, Feast will default to local materialization (pulling all records into memory to materialize).

Users who want to have their offline store support scalable batch materialization for online use cases (detailed in this [RFC](https://docs.google.com/document/d/1J7XdwwgQ9dY_uoV9zkRVGQjK9Sy43WISEW6D5V9qzGo/edit#heading=h.9gaqqtox9jg6)) will also need to implement `to_remote_storage` to distribute the reading and writing of offline store records to blob storage (such as S3). This may be used by a custom [Materialization Engine](https://github.com/feast-dev/feast/blob/master/sdk/python/feast/infra/materialization/batch_materialization_engine.py#L72) to parallelize the materialization of data by processing it in chunks. If this is not implemented, Feast will default to local materialization (pulling all records into memory to materialize).

achals · 2022-07-20T19:34:25Z

docs/how-to-guides/adding-a-new-offline-store.md

 Even if you have created the `OfflineStore` class in a separate repo, you can still test your implementation against the Feast test suite, as long as you have Feast as a submodule in your repo. In the Feast submodule, we can run all the unit tests with:

+In order to test against the test suite, you need to create a custom `DataSourceCreator`. This class will need to implement our testing infrastructure methods, `create_data_source` and `created_saved_dataset_destination`. `create_data_source` creates a datasource for testing from the dataframe given that will register the dataframe with your offline store and return a datasource pointing to that location. See `BigQueryDataSourceCreator` for an implementation of a datawarehouse data source creator. **Saved datasets** are special datasets used for data validation and is Feast's way of snapshotting your data for future data validation.


Suggested change

In order to test against the test suite, you need to create a custom `DataSourceCreator`. This class will need to implement our testing infrastructure methods, `create_data_source` and `created_saved_dataset_destination`. `create_data_source` creates a datasource for testing from the dataframe given that will register the dataframe with your offline store and return a datasource pointing to that location. See `BigQueryDataSourceCreator` for an implementation of a datawarehouse data source creator. **Saved datasets** are special datasets used for data validation and is Feast's way of snapshotting your data for future data validation.

In order to test against the test suite, you need to create a custom `DataSourceCreator`. This class will need to implement our testing infrastructure methods, `create_data_source` and `created_saved_dataset_destination`. `create_data_source` should create a datasource forbased on the dataframe passed in. It may be implemented by uploading the contents of the dataframe into the offline store and returning a datasource object pointing to that location. See `BigQueryDataSourceCreator` for an implementation of a data source creator. **Saved datasets** are special datasets used for data validation and is Feast's way of snapshotting your data for future data validation.

achals · 2022-07-20T19:36:14Z

docs/how-to-guides/adding-a-new-offline-store.md

 ```
+This should run the unit tests and the unit tests should all pass. Please add unit tests for your data source that test out basic functionality of reading and writing to and from the datasource. This should just be class level functionality that ensures that the methods you implemented for the OfflineStore and the DataSource associated with it work as expected. In order to be approved to merge into Feast, these unit tests should all pass and demonstrate that the DataSource works as intended.


Suggested change

This should run the unit tests and the unit tests should all pass. Please add unit tests for your data source that test out basic functionality of reading and writing to and from the datasource. This should just be class level functionality that ensures that the methods you implemented for the OfflineStore and the DataSource associated with it work as expected. In order to be approved to merge into Feast, these unit tests should all pass and demonstrate that the DataSource works as intended.

This command runs the python unit tests. It's required that unit tests should all pass for contributed components.

Contributors should add unit tests for contributed data source that test out basic functionality of reading and writing to and from the datasource. This should just be class level functionality that ensures that the methods you implemented for the OfflineStore and the DataSource associated with it work as expected. In order to be approved to merge into Feast, these unit tests should all pass and demonstrate that the DataSource works as intended.

test out basic functionality of reading and writing to and from the datasource

This actually is not clear at all. Do you mean the test should write from say bigquery? How is that a unit test then?

Hm, I guess most datasources will not have unit tests then. I guess as long as the offline source passes the integration tests, we can trust the store.

achals · 2022-07-20T19:39:54Z

docs/how-to-guides/adding-a-new-offline-store.md

+
+Remember to add documentation for your offline store. This should be added to `docs/reference/offline-stores/` and `docs/reference/data-sources/`. You should also add a reference in `docs/reference/data-sources/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other offline stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the datasource is in alpha development. Please also add some documentation on what the data model is for the specific online store for more clarity.
+
+Finally, add the python code docs by running to document the functions:


This is not necessary right? i.e. the docs are generated automatically when changes are merged, we just need to make sure they are referenced sdk/python/docs/index.rst.

Ok let me update.

achals · 2022-07-20T19:40:32Z

docs/how-to-guides/adding-support-for-a-new-online-store.md

 ```

-The universal tests, which are integration tests specifically intended to test offline and online stores, can be run with:
+This should run the unit tests and the unit tests should all pass. Please add unit tests for your online store that test out basic functionality of reading and writing to and from the online store. This should just be class level functionality that ensures that the methods you implemented for the OnlineStore work as expected. In order to be approved to merge into Feast, these unit tests should all pass and demonstrate that the DataSource works as intended.


Same change and questions as the offline store.

Removed this section entirely. Will basically point to universal tests only to validate the datasource

adchia · 2022-07-20T20:28:04Z

docs/how-to-guides/adding-support-for-a-new-online-store.md

 make test-python-universal
 ```

-The unit tests should succeed, but the universal tests will likely fail. The tests are parametrized based on the `FULL_REPO_CONFIGS` variable defined in `sdk/python/tests/integration/feature_repos/repo_configuration.py`. To overwrite these configurations, you can simply create your own file that contains a `FULL_REPO_CONFIGS`, and point Feast to that file by setting the environment variable `FULL_REPO_CONFIGS_MODULE` to point to that file. In this repo, the file that overwrites `FULL_REPO_CONFIGS` is `feast_custom_online_store/feast_tests.py`, so you would run
+A sample `FULL_REPO_CONFIGS_MODULE` looks something like this:


would follow the pattern of the other code snippets, which also include a "file title" that shows where this lives

See the example of using:
{% code title="feast_custom_offline_store/file.py" %}
... your code
{% endcode %}

that we did before

adchia

took a pass through the offline store only, but I imagine most of the comments also translate to the online store version

adchia · 2022-07-21T01:01:43Z

docs/how-to-guides/adding-support-for-a-new-online-store.md

+
+Remember to add the documentation for your online store. This should be added to `docs/reference/online-stores/`. You should also add a reference in `docs/reference/online-stores/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other online stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the online store is in alpha development.
+
+Finally, add the python code docs by running to document the functions by making sure the classes are being referenced by `sdk/python/docs/index.rst`. An example of this below:


you didn't add the make command here?

Achal mentioned that that is no longer needed and to just update the rst file instead.

adchia · 2022-07-21T01:03:02Z

docs/how-to-guides/adding-a-new-offline-store.md

 The OfflineStore class contains a couple of methods to read features from the offline store. Unlike the OnlineStore class, Feast does not manage any infrastructure for the offline store.&#x20;

 There are two methods that deal with reading data from the offline stores`get_historical_features`and `pull_latest_from_table_or_query`.

 * `pull_latest_from_table_or_query` is invoked when running materialization (using the `feast materialize` or `feast materialize-incremental` commands, or the corresponding `FeatureStore.materialize()` method. This method pull data from the offline store, and the `FeatureStore` class takes care of writing this data into the online store.
 * `get_historical_features` is invoked when reading values from the offline store using the `FeatureStore.get_historical_features()` method. Typically, this method is used to retrieve features when training ML models.
 * `pull_all_from_table_or_query` is a method that pulls all the data from an offline store from a specified start date to a specified end date.
+* `write_logged_features` is a method that takes a pyarrow table or a path that points to a parquet file and writes the data to a defined source defined by `LoggingSource` and `LoggingConfig`. This method is **optional** since it is only used internally for **SavedDatasets**.
+* `offline_write_batch` is a method that supports directly pushing a pyarrow table to a feature view. Given a feature view with a specific schema, this function should write the pyarrow table to the batch source defined. More details about the push api can be found [here](docs/reference/data-sources/push.md).


weird that you only have "write_logged_features" as optional when really pretty much everything is optional except for get_historical_features

Would have the "optional" methods towards the end.

I just treated "optional" as any methods that are labeled abstract even though they have a "pass" I still think the decorator necessitates implementation. I added optional to offline_write batch though.

adchia · 2022-07-21T01:11:28Z

docs/how-to-guides/adding-a-new-offline-store.md

@@ -23,16 +23,25 @@ The process for using a custom offline store consists of 4 steps:
 &#x20;OfflineStore class names must end with the OfflineStore suffix!
 {% endhint %}

+### Contrib
+
+Generally, new offline stores should go in the contrib folder and should have warnings within the classes that state the offline store is in alpha status. The contrib folder is designated for community contributions that are not fully maintained by Feast maintainers and may contain potential instability and have API changes. It is recommended to add warnings to users that the offline store functionality is still in alpha development and is not fully stable. In order to be classified as fully stable and be moved to the main offline store folder, the offline store should integrate with the full integration test suite in Feast and pass all of the test cases.


This is generally pretty hard to read imo. Can you make this simpler and more scannable?

e.g.

Contrib offline stores

New offline stores go in sdk/python/feast/infra/offline_stores/contrib/.

What is a contrib plugin?

Not guaranteed to implement all interface methods

Not guaranteed to be stable

Should have warnings for users to indicate this is contrib

How do I make a contrib plugin an "official" plugin?
To move an offline store plugin out of contrib, you need:

GitHub actions (i.e. make test-python-integration) is setup to run all tests against the offline store and pass.

At least two contributors own the plugin (ideally tracked in an OWNERS / CODEOWNERS)

adchia · 2022-07-21T01:12:05Z

docs/how-to-guides/adding-a-new-offline-store.md

+### Contrib
+
+Generally, new offline stores should go in the contrib folder and should have warnings within the classes that state the offline store is in alpha status. The contrib folder is designated for community contributions that are not fully maintained by Feast maintainers and may contain potential instability and have API changes. It is recommended to add warnings to users that the offline store functionality is still in alpha development and is not fully stable. In order to be classified as fully stable and be moved to the main offline store folder, the offline store should integrate with the full integration test suite in Feast and pass all of the test cases.
+
 The OfflineStore class contains a couple of methods to read features from the offline store. Unlike the OnlineStore class, Feast does not manage any infrastructure for the offline store.&#x20;

 There are two methods that deal with reading data from the offline stores`get_historical_features`and `pull_latest_from_table_or_query`.


this seems to contradict the text you have below

docs/how-to-guides/adding-a-new-offline-store.md

adchia · 2022-07-21T01:20:49Z

docs/how-to-guides/adding-a-new-offline-store.md

+
+### Dependencies
+
+Finally, if your offline store requires special packages, add them to our `sdk/python/setup.py` under a new `<OFFLINE>_STORE_REQUIRED` list with the packages and add it to the setup script so that if your offline store is needed, users can install the necessary python packages. These packages should be defined as extras so that they are not installed by users by default.


why is offline in brackets? shouldn't it be <offline_store> ?

adchia · 2022-07-21T01:21:18Z

docs/how-to-guides/adding-a-new-offline-store.md

+
+### Dependencies
+
+Finally, if your offline store requires special packages, add them to our `sdk/python/setup.py` under a new `<OFFLINE>_STORE_REQUIRED` list with the packages and add it to the setup script so that if your offline store is needed, users can install the necessary python packages. These packages should be defined as extras so that they are not installed by users by default.


Remove Finally since this isn't the last step

adchia · 2022-07-21T01:21:54Z

docs/how-to-guides/adding-a-new-offline-store.md

+
+### Dependencies
+
+Finally, if your offline store requires special packages, add them to our `sdk/python/setup.py` under a new `<OFFLINE>_STORE_REQUIRED` list with the packages and add it to the setup script so that if your offline store is needed, users can install the necessary python packages. These packages should be defined as extras so that they are not installed by users by default.


Also, would remove the "if your offline store requires special packages" part. Every offline store will need to pull in some deps to support a new offline store

adchia · 2022-07-21T01:22:21Z

docs/how-to-guides/adding-a-new-offline-store.md

+
+### Add Documentation
+
+Remember to add documentation for your offline store. This should be added to `docs/reference/offline-stores/` and `docs/reference/data-sources/`. You should also add a reference in `docs/reference/data-sources/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other offline stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the datasource is in alpha development. Please also add some documentation on what the data model is for the specific online store for more clarity.


nit: Convert this to individual steps they can follow instead of many sentences

docs/how-to-guides/adding-a-new-offline-store.md

adchia

again, comments mostly translate to the online store guide too

adchia · 2022-07-22T17:05:10Z

docs/how-to-guides/adding-a-new-offline-store.md

+- Not guaranteed to be stable.
+- Should have warnings for users to indicate this is a contrib plugin that is not maintained by the maintainers.
+
+#### How do I make a contrib plugin and "official" plugin?


Suggested change

#### How do I make a contrib plugin and "official" plugin?

#### How do I make a contrib plugin an "official" plugin?

adchia · 2022-07-22T17:05:31Z

docs/how-to-guides/adding-a-new-offline-store.md

+
+#### How do I make a contrib plugin and "official" plugin?
+To move an offline store plugin out of contrib, you need:
+- Github actions (i.e `make test-python-integration`) is setup to run all tests against the offline store and pass.


Suggested change

- Github actions (i.e `make test-python-integration`) is setup to run all tests against the offline store and pass.

- GitHub actions (i.e `make test-python-integration`) is setup to run all tests against the offline store and pass.

adchia · 2022-07-22T17:05:38Z

docs/how-to-guides/adding-a-new-offline-store.md

+#### How do I make a contrib plugin and "official" plugin?
+To move an offline store plugin out of contrib, you need:
+- Github actions (i.e `make test-python-integration`) is setup to run all tests against the offline store and pass.
+- At least two contributors own the plugin (ideally tracked in our OWNERS / CODEOWNERS file).


Suggested change

- At least two contributors own the plugin (ideally tracked in our OWNERS / CODEOWNERS file).

- At least two contributors own the plugin (ideally tracked in our `OWNERS` / `CODEOWNERS` file).

adchia · 2022-07-22T17:17:59Z

docs/how-to-guides/adding-support-for-a-new-online-store.md

+
+
+2. The universal tests, which are integration tests specifically intended to test offline and online stores, should be run against Feast to ensure that the Feast APIs works with your online store.
+    - To run the integration tests, you must parametrize the integration test suite based on the `FULL_REPO_CONFIGS` variable defined in `sdk/python/tests/integration/feature_repos/repo_configuration.py` to use your own custom online store.


This is kind of confusing to me. I don't think users should try to "run the integration tests" since that is for official plugins.

It's useful context to understand the second bullet though. Would rephrase that Feast parametrizes integration tests using the FULL_REPO_CONFIGS variable, which you overwrite to enable testing for the contrib offline store

adchia · 2022-07-22T17:22:30Z

sdk/python/tests/unit/infra/test_key_encoding_utils.py

@@ -9,14 +9,14 @@ def test_serialize_entity_key():
    # Should be fine
    serialize_entity_key(
        EntityKeyProto(
-            join_keys=["user"], entity_values=[ValueProto(int64_val=int(2 ** 15))]
+            join_keys=["user"], entity_values=[ValueProto(int64_val=int(2**15))]


rebase against master?

Signed-off-by: Kevin Zhang <[email protected]>

adchia

/lgtm

feast-ci-bot · 2022-07-22T21:41:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adchia, kevjumba

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [adchia,kevjumba]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

feast-ci-bot added approved size/L labels Jul 15, 2022

kevjumba added the ok-to-test label Jul 15, 2022

achals reviewed Jul 19, 2022

View reviewed changes

docs/how-to-guides/adding-a-new-offline-store.md Outdated Show resolved Hide resolved

kevjumba force-pushed the update_offline_store_docs branch from a194832 to 36fb89c Compare July 19, 2022 23:01

adchia reviewed Jul 20, 2022

View reviewed changes

adchia self-assigned this Jul 20, 2022

kevjumba force-pushed the update_offline_store_docs branch from 36fb89c to eb028f6 Compare July 20, 2022 17:46

achals reviewed Jul 20, 2022

View reviewed changes

adchia reviewed Jul 20, 2022

View reviewed changes

adchia reviewed Jul 21, 2022

View reviewed changes

adchia reviewed Jul 22, 2022

View reviewed changes

kevjumba added 9 commits July 22, 2022 10:27

Update docs

b70c64b

Signed-off-by: Kevin Zhang <[email protected]>

address review

7343857

Signed-off-by: Kevin Zhang <[email protected]>

Address review

a9f6027

Signed-off-by: Kevin Zhang <[email protected]>

Fix

df11468

Signed-off-by: Kevin Zhang <[email protected]>

Fux

dc26144

Signed-off-by: Kevin Zhang <[email protected]>

Fix

fdfbb75

Signed-off-by: Kevin Zhang <[email protected]>

Fix

6adff74

Signed-off-by: Kevin Zhang <[email protected]>

Fix

c4b2519

Signed-off-by: Kevin Zhang <[email protected]>

Fix

977221f

Signed-off-by: Kevin Zhang <[email protected]>

kevjumba force-pushed the update_offline_store_docs branch from fa0e55d to 977221f Compare July 22, 2022 17:42

kevjumba added 6 commits July 22, 2022 10:46

Fix

846f5fe

Signed-off-by: Kevin Zhang <[email protected]>

Fix

6a85ce2

Signed-off-by: Kevin Zhang <[email protected]>

Fix

d000842

Signed-off-by: Kevin Zhang <[email protected]>

Fix

2694894

Signed-off-by: Kevin Zhang <[email protected]>

Fix

98134bf

Signed-off-by: Kevin Zhang <[email protected]>

Fix

b61f034

Signed-off-by: Kevin Zhang <[email protected]>

kevjumba added 7 commits July 22, 2022 12:58

Fix

69405df

Signed-off-by: Kevin Zhang <[email protected]>

Fix

6d9f507

Signed-off-by: Kevin Zhang <[email protected]>

Fix

76eab5c

Signed-off-by: Kevin Zhang <[email protected]>

Fix

aad3dba

Signed-off-by: Kevin Zhang <[email protected]>

Fix

3945684

Signed-off-by: Kevin Zhang <[email protected]>

Fix

5484ba6

Signed-off-by: Kevin Zhang <[email protected]>

Fix

1cc81e2

Signed-off-by: Kevin Zhang <[email protected]>

kevjumba removed the ok-to-test label Jul 22, 2022

kevjumba added 10 commits July 22, 2022 13:31

Fix

e5ecd48

Signed-off-by: Kevin Zhang <[email protected]>

Fix

e20e6c9

Signed-off-by: Kevin Zhang <[email protected]>

Fix

76353a3

Signed-off-by: Kevin Zhang <[email protected]>

Fix

b071dc7

Signed-off-by: Kevin Zhang <[email protected]>

Fix

6aadfca

Signed-off-by: Kevin Zhang <[email protected]>

Fix

392595a

Signed-off-by: Kevin Zhang <[email protected]>

Fix

ea7c982

Signed-off-by: Kevin Zhang <[email protected]>

Fix

a34cdba

Signed-off-by: Kevin Zhang <[email protected]>

Fix

efa51b1

Signed-off-by: Kevin Zhang <[email protected]>

Fix

5bc74db

Signed-off-by: Kevin Zhang <[email protected]>

adchia approved these changes Jul 22, 2022

View reviewed changes

feast-ci-bot added the lgtm label Jul 22, 2022

adchia enabled auto-merge (squash) July 22, 2022 21:41

feast-ci-bot merged commit 1479519 into feast-dev:master Jul 22, 2022

		@@ -128,6 +130,8 @@ Custom offline stores may need to implement their own instances of the `Retrieva

		The `RetrievalJob` interface exposes two methods - `to_df` and `to_arrow`. The expectation is for the retrieval job to be able to return the rows read from the offline store as a parquet DataFrame, or as an Arrow table respectively.

		Users who want to have their offline store support batch materialization (detailed in this [RFC](https://docs.google.com/document/d/1J7XdwwgQ9dY_uoV9zkRVGQjK9Sy43WISEW6D5V9qzGo/edit#heading=h.9gaqqtox9jg6)) will also need to implement `to_remote_storage` to write distribute the reading and writing of offline store records to a distributed framework. If this functionality is not needed, the RetrievalJob will default to local materialization.


		### Documentation

		Remember to update the documentation for your offline store. This can be found in `docs/reference/offline-stores/` and `docs/reference/data-sources/`. You should also add a reference in `docs/reference/data-sources/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other offline stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the datasource is in alpha development.

		Remember to update the documentation for your offline store. This can be found in `docs/reference/offline-stores/` and `docs/reference/data-sources/`. You should also add a reference in `docs/reference/data-sources/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other offline stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the datasource is in alpha development.


		An example of a full pull request for adding a custom offline store can be found [here](https://github.com/feast-dev/feast/pull/2401).

		@@ -128,6 +151,8 @@ Custom offline stores may need to implement their own instances of the `Retrieva

		The `RetrievalJob` interface exposes two methods - `to_df` and `to_arrow`. The expectation is for the retrieval job to be able to return the rows read from the offline store as a parquet DataFrame, or as an Arrow table respectively.

		Users who want to have their offline store support scalable batch materialization for online use cases (detailed in this [RFC](https://docs.google.com/document/d/1J7XdwwgQ9dY_uoV9zkRVGQjK9Sy43WISEW6D5V9qzGo/edit#heading=h.9gaqqtox9jg6)) will also need to implement `to_remote_storage` to distribute the reading and writing of offline store records to a distributed framework. If this is not implemented, Feast will default to local materialization (pulling all records into memory to materialize).

		Even if you have created the `OfflineStore` class in a separate repo, you can still test your implementation against the Feast test suite, as long as you have Feast as a submodule in your repo. In the Feast submodule, we can run all the unit tests with:

		In order to test against the test suite, you need to create a custom `DataSourceCreator`. This class will need to implement our testing infrastructure methods, `create_data_source` and `created_saved_dataset_destination`. `create_data_source` creates a datasource for testing from the dataframe given that will register the dataframe with your offline store and return a datasource pointing to that location. See `BigQueryDataSourceCreator` for an implementation of a datawarehouse data source creator. Saved datasets are special datasets used for data validation and is Feast's way of snapshotting your data for future data validation.

		```
		This should run the unit tests and the unit tests should all pass. Please add unit tests for your data source that test out basic functionality of reading and writing to and from the datasource. This should just be class level functionality that ensures that the methods you implemented for the OfflineStore and the DataSource associated with it work as expected. In order to be approved to merge into Feast, these unit tests should all pass and demonstrate that the DataSource works as intended.

-This should run the unit tests and the unit tests should all pass. Please add unit tests for your data source that test out basic functionality of reading and writing to and from the datasource. This should just be class level functionality that ensures that the methods you implemented for the OfflineStore and the DataSource associated with it work as expected. In order to be approved to merge into Feast, these unit tests should all pass and demonstrate that the DataSource works as intended.
+This command runs the python unit tests. It's required that unit tests should all pass for contributed components.
+Contributors should add unit tests for contributed data source that test out basic functionality of reading and writing to and from the datasource. This should just be class level functionality that ensures that the methods you implemented for the OfflineStore and the DataSource associated with it work as expected. In order to be approved to merge into Feast, these unit tests should all pass and demonstrate that the DataSource works as intended.


		Remember to add documentation for your offline store. This should be added to `docs/reference/offline-stores/` and `docs/reference/data-sources/`. You should also add a reference in `docs/reference/data-sources/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other offline stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the datasource is in alpha development. Please also add some documentation on what the data model is for the specific online store for more clarity.

		Finally, add the python code docs by running to document the functions:


		Remember to add the documentation for your online store. This should be added to `docs/reference/online-stores/`. You should also add a reference in `docs/reference/online-stores/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other online stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the online store is in alpha development.

		Finally, add the python code docs by running to document the functions by making sure the classes are being referenced by `sdk/python/docs/index.rst`. An example of this below:


		### Dependencies

		Finally, if your offline store requires special packages, add them to our `sdk/python/setup.py` under a new `<OFFLINE>_STORE_REQUIRED` list with the packages and add it to the setup script so that if your offline store is needed, users can install the necessary python packages. These packages should be defined as extras so that they are not installed by users by default.


		### Add Documentation

		Remember to add documentation for your offline store. This should be added to `docs/reference/offline-stores/` and `docs/reference/data-sources/`. You should also add a reference in `docs/reference/data-sources/README.md` and `docs/SUMMARY.md`. Add a new markdown documentation and document the functions similar to how the other offline stores are documented. Be sure to cover how to create the datasource and most importantly, what configuration is needed in the `feature_store.yaml` file in order to create the datasource and also make sure to flag that the datasource is in alpha development. Please also add some documentation on what the data model is for the specific online store for more clarity.

	#### How do I make a contrib plugin and "official" plugin?
	#### How do I make a contrib plugin an "official" plugin?

	- Github actions (i.e `make test-python-integration`) is setup to run all tests against the offline store and pass.
	- GitHub actions (i.e `make test-python-integration`) is setup to run all tests against the offline store and pass.

	- At least two contributors own the plugin (ideally tracked in our OWNERS / CODEOWNERS file).
	- At least two contributors own the plugin (ideally tracked in our `OWNERS` / `CODEOWNERS` file).



		2. The universal tests, which are integration tests specifically intended to test offline and online stores, should be run against Feast to ensure that the Feast APIs works with your online store.
		- To run the integration tests, you must parametrize the integration test suite based on the `FULL_REPO_CONFIGS` variable defined in `sdk/python/tests/integration/feature_repos/repo_configuration.py` to use your own custom online store.

chore: Update docs for offline and online stores #2946

chore: Update docs for offline and online stores #2946

Conversation

kevjumba commented Jul 15, 2022

codecov-commenter commented Jul 15, 2022 • edited Loading

Codecov Report

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

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

kevjumba commented Jul 20, 2022

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adchia 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

Contrib offline stores

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

Choose a reason for hiding this comment

adchia 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

Choose a reason for hiding this comment

adchia left a comment

Choose a reason for hiding this comment

feast-ci-bot commented Jul 22, 2022

codecov-commenter commented Jul 15, 2022 •

edited

Loading