Add AzureOpenAiStructureConfig, default azure_deployment
#788
Conversation
vachillo
force-pushed
the
azure_structure_config
branch
2 times, most recently
from
0998f4b
to
9e2a7b5
Compare
collindutter
force-pushed
the
refactor/default-configs
branch
from
b664e94
to
fc427c7
Compare
vachillo
force-pushed
the
azure_structure_config
branch
from
6a96e24
to
649bfc2
Compare
vachillo
force-pushed
the
azure_structure_config
branch
from
649bfc2
to
e2f7f72
Compare
vachillo
force-pushed
the
azure_structure_config
branch
from
3683e70
to
592075e
Compare
vachillo
changed the title
AzureOpenAiStructureConfigAzureOpenAiStructureConfig, default azure_deployment
vachillo
force-pushed
the
azure_structure_config
branch
from
af6f90f
to
2814c04
Compare
| # pull_request_review: | ||
| # types: [submitted] | ||
| pull_request: | ||
| branches: | ||
| - dev | ||
| jobs: | ||
| test: | ||
| if: github.event.review.state == 'APPROVED' | ||
| # if: github.event.review.state == 'APPROVED' |
There was a problem hiding this comment.
Nice hack to get tests running haha. Just need to revert.
vachillo
force-pushed
the
azure_structure_config
branch
from
2814c04
to
0139e14
Compare
| kw_only=True, default=None, metadata={"serializable": False} | ||
| ) | ||
| api_key: str = field(kw_only=True, default=None, metadata={"serializable": False}) | ||
| prompt_driver: AzureOpenAiChatPromptDriver = field( |
There was a problem hiding this comment.
All of the Driver type hints should use the base class so that they can be overridden with non-azure drivers.
| azure_ad_token_provider: Optional[Callable[[], str]] = field( | ||
| kw_only=True, default=None, metadata={"serializable": False} | ||
| ) | ||
| api_key: str = field(kw_only=True, default=None, metadata={"serializable": False}) |
There was a problem hiding this comment.
Missing Optional type hint.
| azure_ad_token_provider: Optional[Callable[[], str]] = field( | ||
| kw_only=True, default=None, metadata={"serializable": False} | ||
| ) | ||
| api_key: str = field(kw_only=True, default=None, metadata={"serializable": False}) |
There was a problem hiding this comment.
We don't have this field anywhere else, wondering if we should add to the base class?
There was a problem hiding this comment.
probably makes sense actually, i dont really see the benefit. the base class wouldn't do anything with the api_key field by default; at best it reduces some code, at worst it obfuscates where that field is getting pulled from. also some config classes don't use api_key at all (aws bedrock drivers/config)
| prompt_driver: AzureOpenAiChatPromptDriver = field( | ||
| default=Factory( | ||
| lambda self: AzureOpenAiChatPromptDriver( | ||
| model="gpt-4o", |
There was a problem hiding this comment.
Is this out of preview yet? If not we should probably use gpt-4-turbo
There was a problem hiding this comment.
would have to be just gpt-4, gpt-4-turbo is a "version" of gpt-4 as far as azure openai is concerned.
| vector_store_driver: A Local Vector Store Driver. | ||
| """ | ||
|
|
||
| azure_endpoint: str = field(kw_only=True, metadata={"serializable": True}) |
There was a problem hiding this comment.
the deployments now default to the model. it doesn't make sense on the top-level of the config because the deployments are per model.
| config=deserialized_config.merge_config({ | ||
| "prompt_driver" : { | ||
| "model": "anthropic.claude-3-sonnet-20240229-v1:0", | ||
| }, | ||
| }), |
There was a problem hiding this comment.
Would be a good place to show off how azure_deployment can be merged in.
There was a problem hiding this comment.
updated on the azure openai example
There was a problem hiding this comment.
Nice work. Let's merge after #794 to make sure we're not missing anything.
collindutter
force-pushed
the
azure_structure_config
branch
from
a0f1dec
to
532dec9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
closes #790
AzureOpenAiStructureConfigazure_deploymentto the modelAzureOpenAiVisionImageQueryDriverserializable=Falseforazure_ad_token