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

Add max_retries and timeout params to all AzureOpenAI classes #7945

Closed
EdoardoAbatiTR opened this issue Jun 27, 2024 · 11 comments · Fixed by #7983, #7988, #7993 or #7994
Closed

Add max_retries and timeout params to all AzureOpenAI classes #7945

EdoardoAbatiTR opened this issue Jun 27, 2024 · 11 comments · Fixed by #7983, #7988, #7993 or #7994
Labels
Contributions wanted! Looking for external contributions good first issue Good for newcomers type:feature New feature or request

Comments

@EdoardoAbatiTR
Copy link

Is your feature request related to a problem? Please describe.

Currently all OpenAI related classes (e.g. OpenAIDocumentEmbedder, OpenAIChatGenerator) can be initialised by setting max_retries and timeout params.

The corresponding AzureOpenAI don't always have the same params.

Describe the solution you'd like

It would be nice to have these params in the AzureOpenAI classes

Describe alternatives you've considered

Subclass AzureOpenAI and create custom components.

Additional context

cc @anakin87 :)

@anakin87 anakin87 added type:feature New feature or request Contributions wanted! Looking for external contributions good first issue Good for newcomers labels Jun 27, 2024
@anakin87
Copy link
Member

Thanks @EdoardoAbatiTR!

I'm also opening this for contributions. In case anyone is interested, let's discuss it here.

@nvzard
Copy link
Contributor

nvzard commented Jul 4, 2024

Hey,

I am new here. Can I assign this to myself?

@anakin87
Copy link
Member

anakin87 commented Jul 4, 2024

@nvzard feel free to work on this. Take a look at the guidelines for contributors.

I would suggest creating one PR for each component to be changed.
E.g., start with AzureOpenAIGenerator and use OpenAIGenerator as an inspiration.

@nvzard
Copy link
Contributor

nvzard commented Jul 5, 2024

@anakin87 thanks for the references 😄

I've made the first PR for AzureOpenAIGenerator, let me know if I missed anything.

Pending: AzureOpenAIDocumentEmbedder, AzureOpenAITextEmbedder, AzureOpenAIChatGenerator

@EdoardoAbatiTR
Copy link
Author

Hi all, thank you for working on this :)

I think this got closed automatically because there was the "Fixes" keyword the previous PR, but it should stay open until the other classes are also updated

@vblagoje
Copy link
Member

vblagoje commented Jul 8, 2024

Ok np @EdoardoAbatiTR, what other classes need to be updated?

@vblagoje vblagoje reopened this Jul 8, 2024
@nvzard
Copy link
Contributor

nvzard commented Jul 8, 2024

@vblagoje AzureOpenAIDocumentEmbedder, AzureOpenAITextEmbedder, AzureOpenAIChatGenerator are left. I'll open a separate PR for each.

I already have a PR for AzureOpenAIChatGenerator #7988 and will open one for the rest of the classes as well.

@ghost
Copy link

ghost commented Jul 8, 2024

If it's okay for everyone. Can I take a couple of classes?
I'm new to open-source, and would love to contribute.

@nvzard
Copy link
Contributor

nvzard commented Jul 8, 2024

@kanjikinsmoke I already have started work for the other classes. I think there are other issues available to work on: https://github.com/deepset-ai/haystack/issues?q=is%3Aopen+is%3Aissue+label%3A%22Contributions+wanted%21%22

@vblagoje
Copy link
Member

vblagoje commented Jul 8, 2024

@kanjikinsmoke we just talked this morning internally about few easy issues that you could potentially work on. I'll ping @julian-risch to get back to you soon

@nvzard
Copy link
Contributor

nvzard commented Jul 9, 2024

This can be reopened since there is one last PR left to be merged: #7993

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment