Repository: sashabaranov/go-openai
Pull request #214 (2023-03-31 09:57:03)

feat: add azure openai support
feat: add azure openai support


~~refactor config:~~
~~1. add `WithXXX` options, allow construct a config like  `openai.NewConfig(...Option)`~~
~~2. make the config field naming more like the `openai` python pkg's, this will make us follow py's changes more easy~~
~~3. add new `NewConfig` method, which will check the config just like the py' pkg (the error message almost the same)~~

resolve https://github.com/sashabaranov/go-openai/issues/142



codecov[bot]: ## [Codecov](https://codecov.io/gh/sashabaranov/go-openai/pull/214?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> :exclamation: No coverage uploaded for pull request base (`master@8e3a046`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#section-missing-base-commit).
> The diff coverage is `33.75%`.

```diff
@@            Coverage Diff            @@
##             master     #214   +/-   ##
=========================================
  Coverage          ?   67.44%           
=========================================
  Files             ?       21           
  Lines             ?      645           
  Branches          ?        0           
=========================================
  Hits              ?      435           
  Misses            ?      165           
  Partials          ?       45           
```


| [Impacted Files](https://codecov.io/gh/sashabaranov/go-openai/pull/214?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [api.go](https://codecov.io/gh/sashabaranov/go-openai/pull/214?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-YXBpLmdv) | `61.25% <29.62%> (ø)` | |
| [config.go](https://codecov.io/gh/sashabaranov/go-openai/pull/214?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y29uZmlnLmdv) | `38.18% <35.84%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

ttys3: > LGTM, there is one authorization that needs to be revised. Also the /chat/completions API does not exist for the current stable API version `api-version`

authorization for non stream request just fixed.

about the /chat/completions API, we are using the `preview` version. I did not aware that -_-

what can we do about the `stable` version?  add a check and just return an not supported error ? 


tao12345666333: stable API version 2022-12-01 
https://github.com/Azure/azure-rest-api-specs/blob/main/specification/cognitiveservices/data-plane/AzureOpenAI/inference/stable/2022-12-01/inference.json

This API includes a prompt field, if we want to support it, we can just deal with this field.

Or we can just declare we only support 2023-03-15-preview api version

ttys3: currently, I follow the `openai` python package's logic just use the api type to detect the auth method like:

```go
if c.config.APIType == APITypeAzure {
// use api-key method
} else {
// user bear token
}
```

I think maybe it is better to add a `AuthMethod` option to the config ?

like
```go
type AuthMethod interface {
	SetupAuth(req *http.Request, apiKey string)
}
```

then impl two common :

```go
type GenericAuth struct {
	KeyName string
}

func (a *GenericAuth) SetupAuth(req *http.Request, apiKey string) {
	req.Header.Set(a.KeyName, apiKey)
}

type BearTokenAuth struct{}

func (a *BearTokenAuth) SetupAuth(req *http.Request, apiKey string) {
	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", apiKey))
}
```

and we can have the default:

```
var AzureAPIKeyAuth = &GenericAuth{KeyName: "api-key"}

var OpenAITokenAuth = &BearTokenAuth{}
```

sashabaranov: Thank you so much for the PR!
sashabaranov: A couple of general comments:

> 1. add `WithXXX` options, allow construct a config like  `openai.NewConfig(...Option)`

We are actively avoiding this style of configuration as discussed in https://github.com/sashabaranov/go-openai/pull/79.

> 2. make the config field naming more like the `openai` python pkg's, this will make us follow py's changes more easy

While making this package close to python's, it is less of a priority than not breaking API for existing clients. I don't think renaming `Config` fields and `NewConfig` with a returned error are justified given the API non-breakage goal.

ttys3: > 



> A couple of general comments:
> 
> > 1. add `WithXXX` options, allow construct a config like  `openai.NewConfig(...Option)`
> 
> We are actively avoiding this style of configuration as discussed in #79.
> 
> > 2. make the config field naming more like the `openai` python pkg's, this will make us follow py's changes more easy
> 
> While making this package close to python's would be a good thing, it is less of a priority than not breaking API for existing clients. I don't think renaming `Config` fields and `NewConfig` with a returned error is justified given the API non-breakage goal.

1. it's just a kind of  common practice, many project use this style. I can accept that, if you don't like it.

2. if we do not check the error in the Config constructor, then it seems inappropriate to check for configuration errors elsewhere.  we just ignore the error checking， Leave the blame to the server and have the user check for errors returned by the server ？

ttys3: just add error check for `CreateChatCompletionStream`,

seems there's other thing need fix:

```
=== RUN   TestCompletionsStreamWrongModel
    stream_test.go:29: CreateCompletion should return ErrCompletionUnsupportedModel, but returned: Post "http://localhost/v1/completions": dial tcp [::1]:80: connect: connection refused
--- FAIL: TestCompletionsStreamWrongModel (0.00s)
```
sashabaranov: > 1. it's just a kind of common practice, many project use this style. I can accept that, if you don't like it.

Exactly, there are [multiple](https://asankov.dev/blog/2022/01/29/different-ways-to-initialize-go-structs/#variadic-function-constructor) common ways to pass the configuration, this library prefers using a single config struct. 

> 2. if we do not check the error in the Config constructor, then it seems inappropriate to check for configuration errors elsewhere. we may need just ignore the error checking， Leave the blame to the server and have the user check for errors returned by the server ？

This PR checks for three errors:

- **Empty API key** — I think it's fine not to check this during the configuration
- **Whether API type is supported** – just type checking should be enough here if we use iota-enum for APIType
- **Empty APIVersion** — it might make sense to have a `DefaultAzureConfig(...)` that ensures all the nitty-gritty details are specified

```go
if cfg.APIKey == "" {
	return ClientConfig{}, fmt.Errorf("api key is required")
}

if _, ok := supportedAPIType[cfg.APIType]; !ok {
	return ClientConfig{}, fmt.Errorf("unsupported API type %s", cfg.APIType)
}

if cfg.APIType == APITypeAzure || cfg.APIType == APITypeAzureAD {
	if cfg.APIVersion == "" {
		return ClientConfig{}, fmt.Errorf("an API version is required for the Azure API type")
	}
}
```
ttys3: OK.
I updated commits. and it is a mininal patch now. it is non-breaking.

try to fix the `CreateChatCompletionStream` not detect http error code issue, but which cause `ErrorAccumulatorWriteErrors` tests fail.

I think it's better deal this with other PR.

PTAL

sashabaranov: Excellent, ready to merge once codecov issues are fixed 🙌🏻  

Patch:
diff --git a/api.go b/api.go
index 00d6d35..2c978bc 100644
--- a/api.go
+++ b/api.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"strings"
 )
 
 // Client is OpenAI GPT-3 API client.
@@ -39,7 +40,13 @@ func NewOrgClient(authToken, org string) *Client {
 
 func (c *Client) sendRequest(req *http.Request, v interface{}) error {
 	req.Header.Set("Accept", "application/json; charset=utf-8")
-	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.config.authToken))
+	// Azure API Key authentication
+	if c.config.APIType == APITypeAzure {
+		req.Header.Set(AzureAPIKeyHeader, c.config.authToken)
+	} else {
+		// OpenAI or Azure AD authentication
+		req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.config.authToken))
+	}
 
 	// Check whether Content-Type is already set, Upload Files API requires
 	// Content-Type == multipart/form-data
@@ -83,6 +90,15 @@ func (c *Client) sendRequest(req *http.Request, v interface{}) error {
 }
 
 func (c *Client) fullURL(suffix string) string {
+	// /openai/deployments/{engine}/chat/completions?api-version={api_version}
+	if c.config.APIType == APITypeAzure || c.config.APIType == APITypeAzureAD {
+		baseURL := c.config.BaseURL
+		baseURL = strings.TrimRight(baseURL, "/")
+		return fmt.Sprintf("%s/%s/%s/%s%s?api-version=%s",
+			baseURL, azureAPIPrefix, azureDeploymentsPrefix, c.config.Engine, suffix, c.config.APIVersion)
+	}
+
+	// c.config.APIType == APITypeOpenAI || c.config.APIType == ""
 	return fmt.Sprintf("%s%s", c.config.BaseURL, suffix)
 }
 
@@ -100,7 +116,14 @@ func (c *Client) newStreamRequest(
 	req.Header.Set("Accept", "text/event-stream")
 	req.Header.Set("Cache-Control", "no-cache")
 	req.Header.Set("Connection", "keep-alive")
-	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.config.authToken))
 
+	// https://learn.microsoft.com/en-us/azure/cognitive-services/openai/reference#authentication
+	// Azure API Key authentication
+	if c.config.APIType == APITypeAzure {
+		req.Header.Set(AzureAPIKeyHeader, c.config.authToken)
+	} else {
+		// OpenAI or Azure AD authentication
+		req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", c.config.authToken))
+	}
 	return req, nil
 }
diff --git a/config.go b/config.go
index e09c256..52e1efc 100644
--- a/config.go
+++ b/config.go
@@ -5,28 +5,61 @@ import (
 )
 
 const (
-	apiURLv1                       = "https://api.openai.com/v1"
+	openaiAPIURLv1                 = "https://api.openai.com/v1"
 	defaultEmptyMessagesLimit uint = 300
+
+	azureAPIPrefix         = "openai"
+	azureDeploymentsPrefix = "deployments"
+)
+
+type APIType string
+
+const (
+	APITypeOpenAI  APIType = "OPEN_AI"
+	APITypeAzure   APIType = "AZURE"
+	APITypeAzureAD APIType = "AZURE_AD"
 )
 
+const AzureAPIKeyHeader = "api-key"
+
 // ClientConfig is a configuration of a client.
 type ClientConfig struct {
 	authToken string
 
-	HTTPClient *http.Client
+	BaseURL    string
+	OrgID      string
+	APIType    APIType
+	APIVersion string // required when APIType is APITypeAzure or APITypeAzureAD
+	Engine     string // required when APIType is APITypeAzure or APITypeAzureAD
 
-	BaseURL string
-	OrgID   string
+	HTTPClient *http.Client
 
 	EmptyMessagesLimit uint
 }
 
 func DefaultConfig(authToken string) ClientConfig {
 	return ClientConfig{
+		authToken: authToken,
+		BaseURL:   openaiAPIURLv1,
+		APIType:   APITypeOpenAI,
+		OrgID:     "",
+
 		HTTPClient: &http.Client{},
-		BaseURL:    apiURLv1,
+
+		EmptyMessagesLimit: defaultEmptyMessagesLimit,
+	}
+}
+
+func DefaultAzureConfig(apiKey, baseURL, engine string) ClientConfig {
+	return ClientConfig{
+		authToken:  apiKey,
+		BaseURL:    baseURL,
 		OrgID:      "",
-		authToken:  authToken,
+		APIType:    APITypeAzure,
+		APIVersion: "2023-03-15-preview",
+		Engine:     engine,
+
+		HTTPClient: &http.Client{},
 
 		EmptyMessagesLimit: defaultEmptyMessagesLimit,
 	}


Test patch:
diff --git a/api_internal_test.go b/api_internal_test.go
new file mode 100644
index 0000000..83dcafc
--- /dev/null
+++ b/api_internal_test.go
@@ -0,0 +1,133 @@
+package openai
+
+import (
+	"context"
+	"testing"
+)
+
+func TestOpenAIFullURL(t *testing.T) {
+	cases := []struct {
+		Name   string
+		Suffix string
+		Expect string
+	}{
+		{
+			"ChatCompletionsURL",
+			"/chat/completions",
+			"https://api.openai.com/v1/chat/completions",
+		},
+		{
+			"CompletionsURL",
+			"/completions",
+			"https://api.openai.com/v1/completions",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.Name, func(t *testing.T) {
+			az := DefaultConfig("dummy")
+			cli := NewClientWithConfig(az)
+			actual := cli.fullURL(c.Suffix)
+			if actual != c.Expect {
+				t.Errorf("Expected %s, got %s", c.Expect, actual)
+			}
+			t.Logf("Full URL: %s", actual)
+		})
+	}
+}
+
+func TestRequestAuthHeader(t *testing.T) {
+	cases := []struct {
+		Name      string
+		APIType   APIType
+		HeaderKey string
+		Token     string
+		Expect    string
+	}{
+		{
+			"OpenAIDefault",
+			"",
+			"Authorization",
+			"dummy-token-openai",
+			"Bearer dummy-token-openai",
+		},
+		{
+			"OpenAI",
+			APITypeOpenAI,
+			"Authorization",
+			"dummy-token-openai",
+			"Bearer dummy-token-openai",
+		},
+		{
+			"AzureAD",
+			APITypeAzureAD,
+			"Authorization",
+			"dummy-token-azure",
+			"Bearer dummy-token-azure",
+		},
+		{
+			"Azure",
+			APITypeAzure,
+			AzureAPIKeyHeader,
+			"dummy-api-key-here",
+			"dummy-api-key-here",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.Name, func(t *testing.T) {
+			az := DefaultConfig(c.Token)
+			az.APIType = c.APIType
+
+			cli := NewClientWithConfig(az)
+			req, err := cli.newStreamRequest(context.Background(), "POST", "/chat/completions", nil)
+			if err != nil {
+				t.Errorf("Failed to create request: %v", err)
+			}
+			actual := req.Header.Get(c.HeaderKey)
+			if actual != c.Expect {
+				t.Errorf("Expected %s, got %s", c.Expect, actual)
+			}
+			t.Logf("%s: %s", c.HeaderKey, actual)
+		})
+	}
+}
+
+func TestAzureFullURL(t *testing.T) {
+	cases := []struct {
+		Name    string
+		BaseURL string
+		Engine  string
+		Expect  string
+	}{
+		{
+			"AzureBaseURLWithSlashAutoStrip",
+			"https://httpbin.org/",
+			"chatgpt-demo",
+			"https://httpbin.org/" +
+				"openai/deployments/chatgpt-demo" +
+				"/chat/completions?api-version=2023-03-15-preview",
+		},
+		{
+			"AzureBaseURLWithoutSlashOK",
+			"https://httpbin.org",
+			"chatgpt-demo",
+			"https://httpbin.org/" +
+				"openai/deployments/chatgpt-demo" +
+				"/chat/completions?api-version=2023-03-15-preview",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.Name, func(t *testing.T) {
+			az := DefaultAzureConfig("dummy", c.BaseURL, c.Engine)
+			cli := NewClientWithConfig(az)
+			// /openai/deployments/{engine}/chat/completions?api-version={api_version}
+			actual := cli.fullURL("/chat/completions")
+			if actual != c.Expect {
+				t.Errorf("Expected %s, got %s", c.Expect, actual)
+			}
+			t.Logf("Full URL: %s", actual)
+		})
+	}
+}