Skip to content

Conversation

@are-ces
Copy link
Contributor

@are-ces are-ces commented Jan 20, 2026

Fixes the failing test by removing the invalid inference_api parameter from OpenAIVectorStoreMixin initialization.

This resolves the TypeError that was occurring in the test.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jan 20, 2026
@are-ces are-ces changed the title Fix test: remove invalid inference_api parameter Fix: remove invalid inference_api parameter Jan 20, 2026
@are-ces are-ces changed the title Fix: remove invalid inference_api parameter fix: remove invalid inference_api parameter Jan 20, 2026
@leseb
Copy link
Collaborator

leseb commented Jan 20, 2026

@are-ces please fix pre-commit

@are-ces are-ces force-pushed the fix-conflicts-mergify branch 2 times, most recently from b1b6852 to 5ab0582 Compare January 20, 2026 11:47
@leseb
Copy link
Collaborator

leseb commented Jan 20, 2026

unclear why the tests are not running

UV_EXTRA_INDEX_URL: ${{ steps.client-config.outputs.uv-extra-index-url }}
UV_INDEX_STRATEGY: ${{ steps.client-config.outputs.uv-extra-index-url && 'unsafe-best-match' || '' }}
run: |
# Unset UV_INDEX_STRATEGY if empty to avoid uv errors
Copy link
Collaborator

Choose a reason for hiding this comment

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

are these changes necessary?

Copy link
Collaborator

@franciscojavierarceo franciscojavierarceo left a comment

Choose a reason for hiding this comment

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

think you should only change tests/unit/core/test_vector_store_config_registration.py please

@are-ces
Copy link
Contributor Author

are-ces commented Jan 21, 2026

I initially added the UV_INDEX_STRATEGY changes to fix a CI error (see this failed run), but have now reverted them per @franciscojavierarceo 's feedback.

Copy link
Collaborator

@mattf mattf left a comment

Choose a reason for hiding this comment

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

remove the changes to uv.lock

@are-ces are-ces force-pushed the fix-conflicts-mergify branch from 3ca06c1 to 82ae221 Compare January 22, 2026 09:14
@are-ces
Copy link
Contributor Author

are-ces commented Jan 22, 2026

Done

@leseb
Copy link
Collaborator

leseb commented Jan 22, 2026

the lock in the release branch is outdated so we either need to update here or with another PR.

@are-ces
Copy link
Contributor Author

are-ces commented Jan 22, 2026

Should I add the updated lock file again?

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

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants