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

feat: add azure openai provider #309

Merged
merged 11 commits into from
May 2, 2023

Conversation

arbreezy
Copy link
Member

@arbreezy arbreezy commented Apr 20, 2023

Closes #117

πŸ“‘ Description

This PR introduces azure openai and get viper to omit empty strings when writing to config files

βœ… Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

β„Ή Additional Information

@arbreezy arbreezy requested review from a team as code owners April 20, 2023 15:46
This was referenced Apr 20, 2023
@matthisholleville
Copy link
Contributor

Hello @arbreezy , shouldn't we update the README to indicate that we have a new AI Backend available?

@arbreezy
Copy link
Member Author

Hello @arbreezy , shouldn't we update the README to indicate that we have a new AI Backend available?

Yes we will add docs, I just didn't have much time yesterday; I want to test it as well before merging.

@roberthstrand
Copy link
Member

Tried it out, and my first observation is that the help text needs to updated to reflect these changes. I had to check the code to find out what to define as the backend.

Secondly, and this might be out of scope for the actual implementation, but if I want to do an analysis with the Azure OpenAI backend after I have authenticated, I would have so explicitly state that I'm using that as a backend, even when I have no other backends in my configuration file. @AlexsJones, this is probably something we can look into in another issue, right?

Third, and more importantly, I get an error when I run analyze with explain. This is the commands I ran:

> go run . auth -u https://rsopenaigptdemo.openai.azure.com/ -e rsk8sgpt -b azureopenai
Using azureopenai as backend AI provider
Enter azureopenai Key: Provider updated
key added

> go run . analyse -b azureopenai -e
   0% |                                                                                                            | (0/3, 0 it/hr) [0s:0s]
Error: failed while calling AI provider azureopenai: Post "/openai/deployments//chat/completions?api-version=2023-03-15-preview": unsupported protocol scheme ""
exit status 1

Signed-off-by: Aris Boutselis <aris.boutselis@senseon.io>
Signed-off-by: Aris Boutselis <aris.boutselis@senseon.io>
@arbreezy
Copy link
Member Author

Tried it out, and my first observation is that the help text needs to updated to reflect these changes. I had to check the code to find out what to define as the backend.

Secondly, and this might be out of scope for the actual implementation, but if I want to do an analysis with the Azure OpenAI backend after I have authenticated, I would have so explicitly state that I'm using that as a backend, even when I have no other backends in my configuration file. @AlexsJones, this is probably something we can look into in another issue, right?

Third, and more importantly, I get an error when I run analyze with explain. This is the commands I ran:

> go run . auth -u https://rsopenaigptdemo.openai.azure.com/ -e rsk8sgpt -b azureopenai
Using azureopenai as backend AI provider
Enter azureopenai Key: Provider updated
key added

> go run . analyse -b azureopenai -e
   0% |                                                                                                            | (0/3, 0 it/hr) [0s:0s]
Error: failed while calling AI provider azureopenai: Post "/openai/deployments//chat/completions?api-version=2023-03-15-preview": unsupported protocol scheme ""
exit status 1

hello @roberthstrand,
Documentation has been added along with some basic checks when running k8sgpt auth to ensure you will use a valid ai provider/backend.
Could you please run again a test with your Azure OpenAI deployment ?

Re: the analyze cli , I will address it, in a different PR; I agree there is no need to specify a backend if it's the only one in your configuration

Thanks in advance :) !

@AlexsJones
Copy link
Member

ready for review?

@arbreezy
Copy link
Member Author

ready for review?

yes !

pkg/util/util.go Outdated Show resolved Hide resolved
cmd/auth/auth.go Show resolved Hide resolved
cmd/serve/serve.go Outdated Show resolved Hide resolved
pkg/util/util.go Outdated Show resolved Hide resolved
Signed-off-by: Aris Boutselis <aris.boutselis@senseon.io>
Signed-off-by: Aris Boutselis <arisboutselis08@gmail.com>
Signed-off-by: Aris Boutselis <arisboutselis08@gmail.com>
README.md Show resolved Hide resolved
cmd/auth/auth.go Outdated Show resolved Hide resolved
cmd/serve/serve.go Show resolved Hide resolved
pkg/ai/azureopenai.go Outdated Show resolved Hide resolved
Signed-off-by: Aris Boutselis <aris.boutselis@senseon.io>
AlexsJones
AlexsJones previously approved these changes May 2, 2023
AlexsJones and others added 3 commits May 2, 2023 18:09
Signed-off-by: Alex Jones <alexsimonjones@gmail.com>
Signed-off-by: Alex Jones <alexsimonjones@gmail.com>
* feat: add additionalLabels to Service Monitor

Signed-off-by: Brad McCoy <bradmccoydev@gmail.com>

* feat: update additionalLabels

Signed-off-by: Brad McCoy <bradmccoydev@gmail.com>

---------

Signed-off-by: Brad McCoy <bradmccoydev@gmail.com>
@arbreezy arbreezy requested a review from AlexsJones May 2, 2023 15:47
Signed-off-by: Aris Boutselis <aris.boutselis@senseon.io>
@AlexsJones AlexsJones enabled auto-merge (squash) May 2, 2023 17:00
@AlexsJones AlexsJones disabled auto-merge May 2, 2023 20:27
@AlexsJones AlexsJones merged commit d8357ce into k8sgpt-ai:main May 2, 2023
7 checks passed
@shubham-uipath
Copy link

is there any documentation change required for this for end users ? wanted to validate this once with my azure openai account .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Allow for support for Azure OpenAI Services
6 participants