Skip to content

Conversation

@astronautas
Copy link
Contributor

@astronautas astronautas commented Oct 15, 2025

What this PR does / why we need it:

As discussed in #5493, from 0.50.0 materialize does not work against Clickhouse offline store, with the reason being pull_all_from_table_or_query missing in the CH store implementation.

The PR adjusts this by adding the method. Also, makes sure some typing tests are not excluded anymore, apparently they pass just fine now.

Which issue(s) this PR fixes:

#5493,

Misc

@astronautas astronautas requested a review from a team as a code owner October 15, 2025 11:57
@franciscojavierarceo franciscojavierarceo changed the title Add missing pull_all_from_table_or_query to Clickhouse offline store fat: Add missing pull_all_from_table_or_query to Clickhouse offline store Oct 15, 2025
@franciscojavierarceo franciscojavierarceo changed the title fat: Add missing pull_all_from_table_or_query to Clickhouse offline store feat: Add missing pull_all_from_table_or_query to Clickhouse offline store Oct 15, 2025
@astronautas
Copy link
Contributor Author

astronautas commented Oct 15, 2025

I ran "make test-python-universal-clickhouse-offline", to make sure CH integration tests pass, they actually failed before adding the change, worked after adding the change, hopefully the same suite runs on CI as well :D.

@astronautas
Copy link
Contributor Author

astronautas commented Oct 15, 2025

Failing unit tests:

  • ERROR sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py::test_registry_apis[\nauth:\n type: oidc\n client_id: feast-integration-client\n client_secret: feast-integration-client-secret\n username: reader_writer\n password: password\n auth_discovery_url: KEYCLOAK_URL_PLACE_HOLDER/realms/master/.well-known/openid-configuration\n-applied_permissions2-tls_mode0] - AssertionError

  • ERROR sdk/python/tests/unit/test_rag_retriever.py - ValueError: libcublas.so.*[0-9] not found in the system path

  • FAILED sdk/python/tests/unit/online_store/test_online_retrieval.py::test_get_online_features - ImportError: Torch is not available or failed to import.
    Original error:
    libcublas.so.*[0-9] not found in the system path ['/home/runner/work/feast/feast/sdk/python', '/home/runner/work/feast/feast', '/opt/hostedtoolcache/Python/3.12.11/x64/lib/python312.zip', '/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12', '/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12/lib-dynload', '/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12/site-packages', '/home/runner/work/feast/feast/sdk/python', '/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12/site-packages/setuptools/_vendor']

Seem to be outside of my changes? 🤔 I see same issues in other latest MRs.

@astronautas
Copy link
Contributor Author

@franciscojavierarceo

@ntkathole
Copy link
Member

Those failures are due to pytorch releasing wheels early than pypi release. I have a fix #5668. Once merged, you can rebase and pipeline issue should fix.

…terialization logic (calling it)

Signed-off-by: lukas.valatka <lukas.valatka@cast.ai>
Copy link
Member

@ntkathole ntkathole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@astronautas
Copy link
Contributor Author

Thanks for fast responses and reviews!

@astronautas
Copy link
Contributor Author

Let's merge? Not sure who can (I don't seem to have the option available).

@ntkathole ntkathole merged commit eeef56e into feast-dev:master Oct 15, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants