Closed cipri-tom closed 1 year ago
🤖
Hello @cipri-tom,
I'm Dosu, an AI assistant here to provide initial support with your LlamaIndex issues. It's nice to meet you! I can help you solve bugs, answer questions, and provide guidance on contributions. While you're waiting for a human maintainer, I'm here to assist in any way I can.
I'm currently reviewing the issue you've reported about the AzureOpenAI class interface. I'll provide you with a more detailed response shortly.
@cipri-tom quickly checked -- My IDE is not autocompleting api_base
but it is completing azure_endpoint
. Tbh I think both are incorrect, it might need to be base_url
?
@cipri-tom if you are able to test and make a PR, that would be amazing, I don't have access to azure at the moment
OH you meant the env var names changed (I think)?
We should probably fetch from both env vars to maintain backwards compatibility?
@logan-markewich wow, thanks for the amazingly fast response. I have to wrap off for the day, I'll test tomorrow with the changes and try a PR.
OH you meant the env var names changed (I think)?
That as well, but it's not blocking for now, as we can pass the init arguments so that the env vars are ignored. The managing of init args are wrong.
My IDE is not autocompleting api_base but it is completing azure_endpoint
That's correct. The problem is lower, inside AzureOpenAI
, here:
🤖
Thank you for bringing this issue to our attention. It seems like the Azure-specific arguments are being incorrectly passed to the base class OpenAI
, which is causing the APIConnectionError
.
To resolve this, you can modify the AzureOpenAI
class constructor to not pass the Azure-specific arguments to the base class. Here's how you can do it:
def __init__(
self,
mode: str = OpenAIEmbeddingMode.TEXT_SEARCH_MODE,
model: str = OpenAIEmbeddingModelType.TEXT_EMBED_ADA_002,
embed_batch_size: int = DEFAULT_EMBED_BATCH_SIZE,
additional_kwargs: Optional[Dict[str, Any]] = None,
api_key: Optional[str] = None,
api_version: Optional[str] = None,
# azure specific
azure_endpoint: Optional[str] = None,
azure_deployment: Optional[str] = None,
deployment_name: Optional[str] = None,
max_retries: int = 10,
callback_manager: Optional[CallbackManager] = None,
**kwargs: Any,
):
azure_endpoint = get_from_param_or_env(
"azure_endpoint", azure_endpoint, "AZURE_OPENAI_ENDPOINT", ""
)
azure_deployment = resolve_from_aliases(
azure_deployment,
deployment_name,
)
super().__init__(
mode=mode,
model=model,
embed_batch_size=embed_batch_size,
additional_kwargs=additional_kwargs,
api_key=api_key,
api_version=api_version,
max_retries=max_retries,
callback_manager=callback_manager,
**kwargs,
)
self.azure_endpoint = azure_endpoint
self.azure_deployment = azure_deployment
In this modification, the Azure-specific arguments are not passed to the base class constructor. Instead, they are directly assigned to the AzureOpenAI
instance after the base class initialization. This should prevent the APIConnectionError
caused by the incorrectly initialized base class.
Please try this modification and let us know if it resolves your issue.
This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.
Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.
If you want to continue the conversation, start your reply with @dosu-bot.
@logan-markewich I spent a bit of time trying to untangle the various classes and inheritance. I don't think I'm up to the task for that PR because I don't have a view on the philosophy of llama_index
. I see we are re-creating OpenAI
and AzureOpenAI
child, both of which exist in openai
package. I suppose the spirit is to provide the LLM
interface on top of the default ones ?
But then we should do exactly what they do in the original classes, probably (merging azure_endpoint
and api_base
in a single variable):
Plus all the other azure_*
stuff, which not passed to OpenAI
class, but managed locally
@cipri-tom - Hi, I started using llama-index last night and came upon this exact issue. Until this is resolved, do you what version prior to this one that will not have this issue? I've tried llama-index-0.8.67 and still getting the same Connection error. Thanks much in advance.
@aymiee you need llama-index==0.8.47
and openai==0.28.1
. Both, otherwise they're not compatible
@cipri-tom @aymiee I thiiiink the latest version of llama-index should be fine to use for azure. I know several people on discord are using it without issues. Plus just today released a few fixes for edge cases.
Maybe swing by over there to get some help with azure
@logan-markewich This has been one of the most challenging libraries because it's evolving so fast. First the Connection error.... then re-installing for the llama-index==0.8.47 and openai==0.28.1, then having to realize that GPTVectorStoreIndex -> VectorStoreIndex.... and the myriad ways of instantiating the AzureOpenAI and AzureOpenAIEmbeddings
Thank you much for following up with me; I did update llama-index and openai finally it is working! I'm ingesting large pdf's on the fly and it worked great.
@aymiee yea apologies for the rapid changes 😅 openai dev day and the new python client they released (which has been its own wild ride)
Hoping things are starting to level off a bit 🙏
For others that arrive here with the same issue. I managed to use Azure OpenAI providing twice some values:
# engine and azure_deployment have the same value
llm = AzureOpenAI(
api_key=os.getenv("OPENAI_API_KEY", ""),
api_version=os.getenv("OPENAI_API_VERSION", ""),
azure_deployment=os.getenv("OPENAI_ENGINE", ""),
azure_endpoint=os.getenv("OPENAI_API_BASE", ""),
engine=os.getenv("OPENAI_ENGINE", ""),
max_tokens=512,
model="gpt-35-turbo",
temperature=0,
)
# Here again, api_base and azure_endpoint have the same values
embed_model = AzureOpenAIEmbedding(
api_base=os.getenv("OPENAI_API_BASE", ""),
api_key=os.getenv("OPENAI_API_KEY", ""),
api_version=os.getenv("OPENAI_API_VERSION", ""),
azure_deployment=os.getenv("OPENAI_EMBEDDINGS_DEPLOYMENT", ""),
azure_endpoint=os.getenv("OPENAI_API_BASE", ""),
)
As pointed out before, the bug is how the arguments are handled. Maybe, those redundant should be just deleted.
@mmartinortiz I don't think azure_deployment
is required. However, azure_endpoint
is required.
Bug Description
The interface of
AzureOpenAI
class has changed last week, replacing the constructor arguments:OPENAI_API_BASE
->AZURE_OPENAI_ENDPOINT
andOPENAI_API_KEY
->AZURE_OPENAI_API_KEY
.When processing these new arguments, they are incorrectly passed to base class
OpenAI
:https://github.com/run-llama/llama_index/commit/60ac17c677f51a5a8c630dd87c01ecc15cfe26e5#diff-f578cf6f0a325ac8e87a0335a7a34a5f68e9523091cb86a7c9240d78a57b1061R98-R99
As a result, the base class is incorrectly initialized, having an empty
api_base
. And therefore all calls fail withAPIConnectionError: Connection error.
, more specificallyUnsupportedProtocol: Request URL is missing an 'http://' or 'https://' protocol.
.See below for traceback
Version
0.8.68
Steps to Reproduce
Relevant Logs/Tracbacks