community: undo azure_ad_access_token breaking change

[efriis]

No description provided.

[vercel]

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 30, 2024 2:04am

[OhNotWilliam]

This fix is very important Pipeline broke, while nothing changed. Please merge this.

For people googling this problem:
Fixes the error "ERROR! Failed because of: AzureSearch.init() missing 1 required positional argument: 'azure_ad_access_token'" in case you are not providing the token as none.

[levalencia]

I can create another PR for this, but the issue is

def __init__(
    self,
    azure_search_endpoint: str,
    azure_search_key: str,
    azure_ad_access_token: Optional[str]    ,
    index_name: str,
    embedding_function: Union[Callable, Embeddings],
    search_type: str = "hybrid",
    semantic_configuration_name: Optional[str] = None,
    fields: Optional[List[SearchField]] = None,
    vector_search: Optional[VectorSearch] = None,
    semantic_configurations: Optional[
        Union[SemanticConfiguration, List[SemanticConfiguration]]

so defaulting it to None, means the order of parameters must change as parameters with default values must be after required paramers, and if the key is required, then the access token is not, and viceversa.

@efriis @baskaryan what do you suggest here?

[baskaryan]

I can create another PR for this, but the issue is

def __init__(
    self,
    azure_search_endpoint: str,
    azure_search_key: str,
    azure_ad_access_token: Optional[str]    ,
    index_name: str,
    embedding_function: Union[Callable, Embeddings],
    search_type: str = "hybrid",
    semantic_configuration_name: Optional[str] = None,
    fields: Optional[List[SearchField]] = None,
    vector_search: Optional[VectorSearch] = None,
    semantic_configurations: Optional[
        Union[SemanticConfiguration, List[SemanticConfiguration]]

so defaulting it to None, means the order of parameters must change as parameters with default values must be after required paramers, and if the key is required, then the access token is not, and viceversa.

@efriis @baskaryan what do you suggest here?

not sure i follow, you're saying only one of azure_search_key and azure_ad_access_token is required? in that case we can update azure_search_key to be Optional[str]. But yea we can't give it a default value bc we'd need to move it then and that's a breaking change

[levalencia]

Well you authenticate with api key or with access token not with both, so that means both need to be optional with default to None?

[levalencia]

I checked master, it was fixed by adding None and changing the order, the PR was not reverted.