Skip to content

fix(aggregate): send near_image over gRPC instead of raising TypeError - #2221

Open
chrikrah wants to merge 1 commit into
weaviate:mainfrom
chrikrah:fix/aggregate-near-media
Open

chrikrah wants to merge 1 commit into
weaviate:mainfrom
chrikrah:fix/aggregate-near-media

Conversation

@chrikrah

Copy link
Copy Markdown

collection.aggregate.near_image(...) raises TypeError on every server from 1.29 up, before it sends anything. This adds the six media fields to __create_request and passes them on to AggregateRequest. No open issue covers it.

_AggregateGRPC.near_media spreads _parse_media(...) into __create_request. That signature has no near_image parameter, and none for audio, video, depth, thermal or imu. AggregateRequest has all six fields. The integration tests for this method carry a skip since img2vec-neural left CI, so no test ran the gRPC path.

Before, at 09c59d0, the new mock test calls the public API:

$ pytest mock_tests/test_aggregate_near_image.py -q
E       TypeError: _AggregateGRPC.__create_request() got an unexpected keyword argument 'near_image'
weaviate/collections/grpc/aggregate.py:102: TypeError
1 failed

After #2197 drops the GraphQL fallback, this path is the only one aggregate.near_image has.

Verification

$ pytest test -q          # Python 3.12.6
548 passed, 1 skipped
$ pytest mock_tests -q
72 passed

Baseline on 09c59d0 is 542 passed, 1 skipped and 71 passed, so the delta is the seven new tests. One mock test goes through collection.aggregate.near_image. The other six check the request for each media type. Reverting grpc/aggregate.py and keeping the tests gives 7 failed, all on the TypeError above. ruff check and ruff format --diff (0.14.7), flake8 and pyright 1.1.399 on weaviate/ report clean on the branch.

# not run: integration/ (needs a server with a media vectorizer)

@g-despot since #2197 routes every aggregation through gRPC, could this one go in alongside it?

…e request

Signed-off-by: Christopher Krah <48338417+chrikrah@users.noreply.github.com>

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@weaviate-git-bot

Copy link
Copy Markdown

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Forum?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants