feat: Clickhouse offline store

[iamhatesz]

What this PR does / why we need it:

This PR adds a new contrib offline store backed by Clickhouse.

Which issue(s) this PR fixes:

Lack of Clickhouse support :)

Misc

The implementation is heavily based on the Postgres store and tested against it. The resulting features were identical to the point that it's possible with two different backends (e.g., different data types).

I added a helper to run integration tests: make test-python-universal-clickhouse-offline. Unfortunately, 3 test cases are failing:

ERROR sdk/python/tests/integration/registration/test_universal_types.py::test_feature_get_historical_features_types_match[TypeTestConfig(feature_dtype='float', feature_is_list=True, has_empty_list=False)-ParameterSet(values=(LOCAL:Clickhouse:RedisOnlineStoreCreator:python_fs:False,), marks=[], id=None)] - TypeError: object of type 'NoneType' has no len()
ERROR sdk/python/tests/integration/registration/test_universal_types.py::test_feature_get_historical_features_types_match[TypeTestConfig(feature_dtype='bool', feature_is_list=True, has_empty_list=False)-ParameterSet(values=(LOCAL:Clickhouse:RedisOnlineStoreCreator:python_fs:False,), marks=[], id=None)] - TypeError: object of type 'NoneType' has no len()
ERROR sdk/python/tests/integration/registration/test_universal_types.py::test_feature_get_historical_features_types_match[TypeTestConfig(feature_dtype='datetime', feature_is_list=True, has_empty_list=False)-ParameterSet(values=(LOCAL:Clickhouse:RedisOnlineStoreCreator:python_fs:False,), marks=[], id=None)] - TypeError: object of type 'NoneType' has no len()

This is because Clickhouse doesn't support Nullable(Array(...)) type. I could have added test_universal_types to the ignore list, but I thought it's worth keeping it on, as many other test cases from this test are passing.

[zerafachris]

Hi @iamhatesz , I patched the requirements on your branch to hopefully fix the errors being thrown by the checks

• Update py3.9-requirements.txt iamhatesz/feast#2
• Update py3.11-requirements.txt iamhatesz/feast#3
• Update py3.11-ci-requirements.txt iamhatesz/feast#4

Merge at your earliest convenience.

Looking forward to using this. ATM, running on PG and would like to move to CH asap

[franciscojavierarceo]

Bunch of stuff failing here, think you also need to rebase. Let me know if you need some help!

[iamhatesz]

@zerafachris @franciscojavierarceo thanks! I will take a look and push some updates.

[iamhatesz]

Hey @franciscojavierarceo! I fixed some issues related to Python 3.9, regenerated lock files and merged the latest changes. Could you please approve the workflows to see if the tests are passing now?

[iamhatesz]

FAILED sdk/python/tests/integration/online_store/test_universal_online.py::test_async_online_retrieval_with_event_timestamps_dynamo[ParameterSet(values=(LOCAL:File:dynamodb:python_fs:False,), marks=[], id=None)] - botocore.exceptions.HTTPClientError: An HTTP Client raised an unhandled exception: Event loop is closed

this failure doesn't seem to be related with the content of this PR.

[miroslawas]

Would love to have this functionality as part of Feast ❤️

[iamhatesz]

Hey @franciscojavierarceo! Could you please help me find a reviewer for this PR?

[franciscojavierarceo]

Yes, reviewing now!

[franciscojavierarceo]

For the failed test cases, can you mark ignore on them for clickhouse? Probably worth adding that Nullable Arrays aren't supported, yeah?

Apologies for the delay on this, frankly, this is embarrassing on my part. Do feel free to tag me directly going forward. I'll get this addressed ASAP, thank you for the contribution. 🙏

[iamhatesz]

Hey @franciscojavierarceo ! Thanks for the review. I hope I addressed all your concerns. I had to add the ignore rule as part of a fixture, since pytest -k doesn't seem to work with complex test case names (e.g., with parentheses). I hope it's not a problem since I based on a similar hack for Redshift.

[masterlexa]

Hello, we are really looking forward to this feature, can you tell us when it will be added?)

[franciscojavierarceo]

lgtm!

[franciscojavierarceo]

@iamhatesz sorry one last conflict needs to be resolved 😞

[iamhatesz]

@franciscojavierarceo merged the latest changes and regenerated lock files with make lock-python-dependencies-all but it also updates all packages to their latest versions. After updating torch (which is completely irrelevant to this PR), two unit tests failed due to missing wheels. Let me know how I can fix this. How do you guys update the lock files if not with that command?

The e2e test failure seems unrelated as well.

[franciscojavierarceo]

@ntkathole @jyejare

[ntkathole]

@franciscojavierarceo merged the latest changes and regenerated lock files with make lock-python-dependencies-all but it also updates all packages to their latest versions. After updating torch (which is completely irrelevant to this PR), two unit tests failed due to missing wheels. Let me know how I can fix this. How do you guys update the lock files if not with that command?

The e2e test failure seems unrelated as well.

@iamhatesz can you please pin torch version 2.2.2 in setup.py and pyproject.toml ? This is happening since PyTorch 2.2.x is the last version that supports macOS x64.

[iamhatesz]

@ntkathole done

[ntkathole]

Looks good!