-
Notifications
You must be signed in to change notification settings - Fork 41
add endpoint_envvar as argument for AsyncEmbeddingModel #36
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
Conversation
gvanrossum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind running the code though black (make format in the repo) and adding a unittest to test/test_embeddings.py?
|
Also would be nice to merge with main before re-submitting. (Let me know if my requirements are starting to feel onerous -- we can make other arrangements, I want you to be a happy contributor.) |
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
…-py into add-endpoint-argument
No problem, clear guidance and feedback is very helpful. Merged back main and added a unit test. |
|
Looks like your tests are failing. The fix is probably simple enough. To check that you've got it, run "make check test format" locally before committing and pushing. |
…ck for none in test
gvanrossum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
W00t! Thanks for persevering. Curious what you're planning to do with typeagent, and how I can help. |
Allow passing of
endpoint_envvarinto constructor and check if provided value matches withmodel_to_embedding_size_and_envvarif entry for model exists.