FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: Add missing pull_all_from_table_or_query to Clickhouse offline store by astronautas · Pull Request #5667 · feast-dev/feast · GitHub

feat: Add missing pull_all_from_table_or_query to Clickhouse offline store - #5667

Merged
ntkathole merged 1 commit into
feast-dev:masterfrom
astronautas:master
Oct 15, 2025
Merged

feat: Add missing pull_all_from_table_or_query to Clickhouse offline store#5667
ntkathole merged 1 commit into
feast-dev:masterfrom
astronautas:master

Conversation

astronautas commented Oct 15, 2025
edited
Loading

Copy link
Copy Markdown
Contributor

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 requested a review from a team as a code owner October 15, 2025 11:57
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 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 commented Oct 15, 2025
edited
Loading

Copy link
Copy Markdown
Contributor Author

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 commented Oct 15, 2025
edited
Loading

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor Author

@franciscojavierarceo

Copy link
Copy Markdown
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>

ntkathole left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

Copy link
Copy Markdown
Contributor Author

Thanks for fast responses and reviews!

Copy link
Copy Markdown
Contributor Author

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

ntkathole merged commit eeef56e into feast-dev:master Oct 15, 2025
17 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL