Hi @qooba. Thanks for your PR.

I'm waiting for a feast-dev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

/ok-to-test

This is great work @qooba! Really surprised to see the implementation so quickly. We're still in the process of collecting feedback on the #1497, given that we may need to support a more scalable ingestion API for real time events. We may need a little bit of time before we can review and merge this code in, but we're super thankful for the contribution!!

One thing that stands out is that we don't have any automated tests for Redis as part of this PR. We'll definitely need to extend the PR to also include an integration test. @qooba do you want to take a stab at that?

One option is https://github.com/testcontainers/testcontainers-python, another is to just mark the tests as integration tests using @pytest.mark.integration and then only run tests on GitHub Actions with a Redis service provisioned

Sure @woop I will extend PR with integration tests.

/retest

For some reason it's not allowing you to run tests until you've actually merged some code into Feast. I'll need to approve every time.

@jklegar I have added sdk/python/tests/test_cli_redis.py and also sdk/python/tests/test_offline_online_store_consistency.py::test_redis_offline_online_store_consistency to check offline consistency with redis. Redis service also added to GitHub action workflow.

The test sdk/python/tests/test_cli_redis.py::test_basic fails because added configuration is not used:

env:
  REDIS_TYPE: REDIS
  REDIS_CONNECTION_STRING: redis:6379,db=0

@jklegar can you help with this ?

@qooba I've added the redis service to pr_integration_tests.yml in master, can you rebase this diff? Once you rebase it'll run the updated action here

@jklegar there will be small change in github action configuration:
b8b0d9f

Additionally macOS-latest doesn't have docker thus tests will never pass can you remove it :)

And commit once again to master then I will rebase.

Thanks @qooba and apologies for the delay here, it should be good to rebase now

Thanks @jklegar rebased and tests are passing now :)

/kind feature

/lgtm