feat: Implement gRPC server to ingest streaming features #3687

mehmettokgoz · 2023-07-17T08:06:40Z

What this PR does / why we need it:

Currently only way to ingest streaming features to Feast is using the Spark/Kafka processor. This processor has several limitations. It retrieves data in batches and forces to run all transformations in Python runtime.

The gRPC server that listens for the streaming features allows users to apply transformation on more powerful streaming engines and send it to Feast then. This feature introduces new streaming sources for Feast, in addition to Kafka and Kinesis.

Our main purpose for the gRPC ingestion service is to integrate Hazelcast streaming engine with Feast. Hazelcast provides various connectors, and supports to process those sources in real-time. This server makes possible to sink transformed data to Feast.

feast-ci-bot · 2023-07-17T08:06:46Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mehmettokgoz
To complete the pull request process, please assign kevjumba after the PR has been reviewed.
You can assign the PR to them by writing /assign @kevjumba in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

sdk/python/feast/infra/contrib/grpc_server.py

mehmettokgoz · 2023-07-27T20:19:44Z

Thanks for review @adchia. Waiting for your answers.

sdk/python/feast/infra/contrib/grpc_server.py

mehmettokgoz · 2023-08-07T15:22:14Z

@adchia addressed review comments. ✅

Changes:

Do not force only one feature view. Proto object contains a feature_view_name field.
Push API is supported.
Health check service is added to server.
grpcio packages are upgraded to 1.56.2
CLI is refactored.

crispin-ki · 2023-08-15T21:42:28Z

I saw a comment about tests failing in this PR in the community notes.

Failing python unit tests should be fixed by #3734. Once that PR is merged you can rebase this branch on top of master and the failing tests should pass.

mehmettokgoz · 2023-08-17T08:49:06Z

Thanks @crispin-ki

Do you have any idea why linter might be failing? The errors are not related to the changes in this PR and I see it's green for your PR.

crispin-ki · 2023-08-17T10:42:29Z

Thanks @crispin-ki

Do you have any idea why linter might be failing? The errors are not related to the changes in this PR and I see it's green for your PR.

Yeah it's a bit weird that these are failing now, seeing as you didn't touch them. I think you have a few options though:

logs for failing workflow (pasted at bottom of this comment too) are all E721s, so you could add E721 here so those errors are now ignored altogether
alternatively you could go to each line that is failing and add # noqa: E721 at the end of line similar to here but with E721 instead, so flake doesn't check that line for E721
alternatively you could go to each line that is failing and change them so flake is happy (in most cases change == to is)

In terms of which is preferred, I think all have pros and cons. I think just go with whichever you feel is most appropriate (or if unsure, might be one for maintainers to decide - they might in fact have a better solution )

Logs for the failing workflow:

cd /home/runner/work/feast/feast/sdk/python; python -m flake8 feast/ tests/
feast/diff/registry_diff.py:[13](https://github.com/feast-dev/feast/actions/runs/5887487203/job/15966956374?pr=3687#step:9:14)6:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
feast/feature_store.py:1109:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
feast/field.py:65:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
feast/infra/offline_stores/bigquery.py:728:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
feast/type_map.py:748:8: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
tests/unit/local_feast_tests/test_local_feature_store.py:472:20: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
tests/unit/test_feature_views.py:183:[16](https://github.com/feast-dev/feast/actions/runs/5887487203/job/15966956374?pr=3687#step:9:17): E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`

mehmettokgoz · 2023-08-17T12:02:45Z

@crispin-ki

I found that it is related to new version of flake8 (which upgrades pycodestyle). Now pycodetype check for EE721 (https://github.com/PyCQA/pycodestyle/blob/main/CHANGES.txt#L12). I set flake version to >=6.0.0,<6.1.0.

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Signed-off-by: Danny C <[email protected]>

adchia · 2023-09-05T08:09:58Z

/lgtm

adchia · 2023-09-05T17:35:25Z

/lgtm

adchia · 2023-09-05T17:36:37Z

/lgtm /approve

Signed-off-by: Danny C <[email protected]>

# [0.34.0](v0.33.0...v0.34.0) (2023-09-07) ### Bug Fixes * Add NUMERIC to bq_to_feast type map ([#3719](#3719)) ([6474b4b](6474b4b)) * Fix python unit tests ([#3734](#3734)) ([e81684d](e81684d)) * Handle unknown postgres source types gracefully ([#3634](#3634)) ([d7041f4](d7041f4)) * Pin protobuf version to avoid seg fault on some machines ([028cc20](028cc20)) * Remove unwanted excessive splitting of gcs path, so expected gcs parquet paths are returned from BigQueryRetrievalJob.to_remote_storage() ([#3730](#3730)) ([f2c5988](f2c5988)) * Run store.plan() only when need it. ([#3708](#3708)) ([7bc7c47](7bc7c47)) * Saved datasets no longer break CLI registry-dump command ([#3717](#3717)) ([f28ccc2](f28ccc2)) * Update py3.8 ci requirements for cython 3.0 release ([#3735](#3735)) ([1695c13](1695c13)) ### Features * Enhance customization of Trino connections when using Trino-based Offline Stores ([#3699](#3699)) ([ed7535e](ed7535e)) * Implement gRPC server to ingest streaming features ([#3687](#3687)) ([a3fcd1f](a3fcd1f))

) * Implemented gRPC server for ingesting streaming features. Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

# [0.34.0](feast-dev/feast@v0.33.0...v0.34.0) (2023-09-07) ### Bug Fixes * Add NUMERIC to bq_to_feast type map ([feast-dev#3719](feast-dev#3719)) ([6474b4b](feast-dev@6474b4b)) * Fix python unit tests ([feast-dev#3734](feast-dev#3734)) ([e81684d](feast-dev@e81684d)) * Handle unknown postgres source types gracefully ([feast-dev#3634](feast-dev#3634)) ([d7041f4](feast-dev@d7041f4)) * Pin protobuf version to avoid seg fault on some machines ([028cc20](feast-dev@028cc20)) * Remove unwanted excessive splitting of gcs path, so expected gcs parquet paths are returned from BigQueryRetrievalJob.to_remote_storage() ([feast-dev#3730](feast-dev#3730)) ([f2c5988](feast-dev@f2c5988)) * Run store.plan() only when need it. ([feast-dev#3708](feast-dev#3708)) ([7bc7c47](feast-dev@7bc7c47)) * Saved datasets no longer break CLI registry-dump command ([feast-dev#3717](feast-dev#3717)) ([f28ccc2](feast-dev@f28ccc2)) * Update py3.8 ci requirements for cython 3.0 release ([feast-dev#3735](feast-dev#3735)) ([1695c13](feast-dev@1695c13)) ### Features * Enhance customization of Trino connections when using Trino-based Offline Stores ([feast-dev#3699](feast-dev#3699)) ([ed7535e](feast-dev@ed7535e)) * Implement gRPC server to ingest streaming features ([feast-dev#3687](feast-dev#3687)) ([a3fcd1f](feast-dev@a3fcd1f))

) * Implemented gRPC server for ingesting streaming features. Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

# [0.34.0](feast-dev/feast@v0.33.0...v0.34.0) (2023-09-07) ### Bug Fixes * Add NUMERIC to bq_to_feast type map ([feast-dev#3719](feast-dev#3719)) ([6474b4b](feast-dev@6474b4b)) * Fix python unit tests ([feast-dev#3734](feast-dev#3734)) ([e81684d](feast-dev@e81684d)) * Handle unknown postgres source types gracefully ([feast-dev#3634](feast-dev#3634)) ([d7041f4](feast-dev@d7041f4)) * Pin protobuf version to avoid seg fault on some machines ([028cc20](feast-dev@028cc20)) * Remove unwanted excessive splitting of gcs path, so expected gcs parquet paths are returned from BigQueryRetrievalJob.to_remote_storage() ([feast-dev#3730](feast-dev#3730)) ([f2c5988](feast-dev@f2c5988)) * Run store.plan() only when need it. ([feast-dev#3708](feast-dev#3708)) ([7bc7c47](feast-dev@7bc7c47)) * Saved datasets no longer break CLI registry-dump command ([feast-dev#3717](feast-dev#3717)) ([f28ccc2](feast-dev@f28ccc2)) * Update py3.8 ci requirements for cython 3.0 release ([feast-dev#3735](feast-dev#3735)) ([1695c13](feast-dev@1695c13)) ### Features * Enhance customization of Trino connections when using Trino-based Offline Stores ([feast-dev#3699](feast-dev#3699)) ([ed7535e](feast-dev@ed7535e)) * Implement gRPC server to ingest streaming features ([feast-dev#3687](feast-dev#3687)) ([a3fcd1f](feast-dev@a3fcd1f))

PR feast-dev#3687 added a spiffy feature to ingest streaming features, but this came along with a large batch of depdencies. Notable this induces a core dependency on `protobuf>=4.21.6` while Feast itself is on `protobuf<4.23.4,>3.20`. This is a fiddly narrow range and excludes all 3.x uses. Signed-off-by: Chris Burroughs <[email protected]>

) * Implemented gRPC server for ingesting streaming features. Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]> Signed-off-by: Attila Toth <[email protected]>

# [0.34.0](feast-dev/feast@v0.33.0...v0.34.0) (2023-09-07) ### Bug Fixes * Add NUMERIC to bq_to_feast type map ([feast-dev#3719](feast-dev#3719)) ([6474b4b](feast-dev@6474b4b)) * Fix python unit tests ([feast-dev#3734](feast-dev#3734)) ([e81684d](feast-dev@e81684d)) * Handle unknown postgres source types gracefully ([feast-dev#3634](feast-dev#3634)) ([d7041f4](feast-dev@d7041f4)) * Pin protobuf version to avoid seg fault on some machines ([028cc20](feast-dev@028cc20)) * Remove unwanted excessive splitting of gcs path, so expected gcs parquet paths are returned from BigQueryRetrievalJob.to_remote_storage() ([feast-dev#3730](feast-dev#3730)) ([f2c5988](feast-dev@f2c5988)) * Run store.plan() only when need it. ([feast-dev#3708](feast-dev#3708)) ([7bc7c47](feast-dev@7bc7c47)) * Saved datasets no longer break CLI registry-dump command ([feast-dev#3717](feast-dev#3717)) ([f28ccc2](feast-dev@f28ccc2)) * Update py3.8 ci requirements for cython 3.0 release ([feast-dev#3735](feast-dev#3735)) ([1695c13](feast-dev@1695c13)) ### Features * Enhance customization of Trino connections when using Trino-based Offline Stores ([feast-dev#3699](feast-dev#3699)) ([ed7535e](feast-dev@ed7535e)) * Implement gRPC server to ingest streaming features ([feast-dev#3687](feast-dev#3687)) ([a3fcd1f](feast-dev@a3fcd1f)) Signed-off-by: Attila Toth <[email protected]>

PR feast-dev#3687 added a spiffy feature to ingest streaming features, but this came along with a large batch of depdencies. Notable this induces a core dependency on `protobuf>=4.21.6` while Feast itself is on `protobuf<4.23.4,>3.20`. This is a fiddly narrow range and excludes all 3.x uses. Signed-off-by: Chris Burroughs <[email protected]>

feast-ci-bot added the size/L label Jul 17, 2023

mehmettokgoz changed the title ~~feat: gRPC server implementation for ingesting streaming features~~ feat: Implement gRPC server to ingest streaming features Jul 17, 2023

adchia reviewed Jul 23, 2023

View reviewed changes

sdk/python/feast/infra/contrib/grpc_server.py Outdated Show resolved Hide resolved

sdk/python/feast/infra/contrib/grpc_server.py Show resolved Hide resolved

mehmettokgoz requested a review from adchia July 27, 2023 20:18

adchia reviewed Aug 3, 2023

View reviewed changes

mehmettokgoz requested a review from adchia August 7, 2023 09:56

mehmettokgoz force-pushed the grpc-server branch from f12dd5a to 43a2c91 Compare August 28, 2023 15:37

adchia force-pushed the grpc-server branch from 43a2c91 to 1c074f0 Compare September 5, 2023 08:05

mehmettokgoz and others added 12 commits September 5, 2023 01:09

Implemented gRPC server for ingesting streaming features.

b235d1a

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

[lint-python]

1b4bf8d

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Fix unmatched gRPC field issue.

809b9eb

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Make max_workers parameter configurable.

5ea58d1

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

[lint-python]

9766c53

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Refactor gRPC server to have similar logic with HTTP server

b92b838

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Refactor CLI.

caed020

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Type parameter.

cd537d9

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Upgrade 3.8 requirements.

d73c3d7

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Configure health check service and update CI dependencies

a5000d8

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

Downgrade flake8 version to prevent E721 checks.

be0010d

Signed-off-by: mehmettokgoz <[email protected]> Signed-off-by: Danny C <[email protected]>

lint

5f73e77

Signed-off-by: Danny C <[email protected]>

adchia force-pushed the grpc-server branch from bc36d7d to 5f73e77 Compare September 5, 2023 08:09

adchia approved these changes Sep 5, 2023 •

edited

Loading

View reviewed changes

adchia approved these changes Sep 5, 2023

View reviewed changes

adchia added approved lgtm labels Sep 5, 2023

adchia added 3 commits September 5, 2023 10:45

fix grpcio pin

392768a

Signed-off-by: Danny C <[email protected]>

fix lint

0c75491

Signed-off-by: Danny C <[email protected]>

Fix linter

068de80

Signed-off-by: Danny C <[email protected]>

adchia merged commit a3fcd1f into feast-dev:master Sep 7, 2023
15 checks passed

cburroughs mentioned this pull request Jan 24, 2024

fix: Move gRPC dependencies to an extra #3900

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement gRPC server to ingest streaming features #3687

feat: Implement gRPC server to ingest streaming features #3687

mehmettokgoz commented Jul 17, 2023

feast-ci-bot commented Jul 17, 2023

mehmettokgoz commented Jul 27, 2023

mehmettokgoz commented Aug 7, 2023

crispin-ki commented Aug 15, 2023

mehmettokgoz commented Aug 17, 2023

crispin-ki commented Aug 17, 2023 •

edited

Loading

mehmettokgoz commented Aug 17, 2023

adchia commented Sep 5, 2023

adchia commented Sep 5, 2023

adchia commented Sep 5, 2023

feat: Implement gRPC server to ingest streaming features #3687

feat: Implement gRPC server to ingest streaming features #3687

Conversation

mehmettokgoz commented Jul 17, 2023

feast-ci-bot commented Jul 17, 2023

mehmettokgoz commented Jul 27, 2023

mehmettokgoz commented Aug 7, 2023

crispin-ki commented Aug 15, 2023

mehmettokgoz commented Aug 17, 2023

crispin-ki commented Aug 17, 2023 • edited Loading

mehmettokgoz commented Aug 17, 2023

adchia commented Sep 5, 2023

adchia commented Sep 5, 2023

adchia commented Sep 5, 2023

crispin-ki commented Aug 17, 2023 •

edited

Loading