Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

comminity[patch]: fix #25575 YandexGPTs for _grpc_metadata #25617

Merged
merged 16 commits into from
Aug 29, 2024

Conversation

mkhludnev
Copy link
Contributor

@mkhludnev mkhludnev commented Aug 21, 2024

it fixes two issues:

YGPTs are broken #25575

File ....conda/lib/python3.11/site-packages/langchain_community/embeddings/yandex.py:211, in _make_request(self, texts, **kwargs)
..
--> 211 res = stub.TextEmbedding(request, metadata=self._grpc_metadata)  # type: ignore[attr-defined]

AttributeError: 'YandexGPTEmbeddings' object has no attribute '_grpc_metadata'

My gut feeling that #23841 is the cause.

I have to drop leading underscore from _grpc_metadata for quickfix, but I just don't know how to do it pydantic enough.

minor issue:

if we use api_key, which is not the best practice the code fails with

File ~/git/...../python3.11/site-packages/langchain_community/embeddings/yandex.py:119, in YandexGPTEmbeddings.validate_environment(cls, values)
...

AttributeError: 'tuple' object has no attribute 'append'
  • Added new integration test. But it requires YGPT env available and active account. I don't know how int tests dis\enabled in CI.
  • added small unit tests with mocks. Should be fine.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 21, 2024
Copy link

vercel bot commented Aug 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Aug 22, 2024 2:23pm

@dosubot dosubot bot added community Related to langchain-community Ɑ: embeddings Related to text embedding models module 🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature labels Aug 21, 2024
@mkhludnev
Copy link
Contributor Author

mkhludnev commented Aug 21, 2024

ok. My unittests requires yandexcloud dependency. Can we add yandexcloud into extended_testing_deps.txt?

@mkhludnev
Copy link
Contributor Author

mkhludnev commented Aug 22, 2024

Changed one test working without yandexcloud dependency. 41dc558
Let me know if it makes sense, I'll move toward two remaining unittests.

@mkhludnev
Copy link
Contributor Author

although it suffer from ModuleNotFoundError: No module named 'grpc'
I'll take one more attempt

@mkhludnev
Copy link
Contributor Author

@tyumentsev4 wdyt?

@mkhludnev mkhludnev changed the title comminity[patch]: fix YandexGPTs for _grpc_metadata comminity[patch]: fix #25575 YandexGPTs for _grpc_metadata Aug 26, 2024
@mkhludnev
Copy link
Contributor Author

@eyurtsev may I ask your attention?

@baskaryan baskaryan merged commit a017f49 into langchain-ai:master Aug 29, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:bug Related to a bug, vulnerability, unexpected error with an existing feature community Related to langchain-community Ɑ: embeddings Related to text embedding models module size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants