Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Adding the User param to the site config so that it can be supported by Azure as an extra param#62950

Merged
arafatkatze merged 4 commits into
mainfrom
adding-user-param-azure
May 28, 2024
Merged

Adding the User param to the site config so that it can be supported by Azure as an extra param#62950
arafatkatze merged 4 commits into
mainfrom
adding-user-param-azure

Conversation

@arafatkatze

Copy link
Copy Markdown
Contributor

See what the field Does

Test plan

Tested locally by setting up extra variable in siteadmin with azure config and it worked perfectly.

@cla-bot cla-bot Bot added the cla-signed label May 28, 2024
@arafatkatze arafatkatze force-pushed the adding-user-param-azure branch from b20650a to c4da6ea Compare May 28, 2024 17:49
Comment thread internal/completions/client/azureopenai/openai.go Outdated

@dominiccooney dominiccooney left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I assume the ,omitempty is all we need to make this a no-op for models who do not use it and sites which do not specify it.

Comment thread schema/site.schema.json Outdated
"type": "string"
},
"user": {
"description": "Its the user field for OpenAI config for both AzureOpenAI and OpenAI",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just "The ...", we don't need "Its ..."

(Its is possessive anyway and definitely wrong.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@arafatkatze arafatkatze enabled auto-merge (squash) May 28, 2024 22:41
@arafatkatze arafatkatze merged commit 8e37fca into main May 28, 2024
@arafatkatze arafatkatze deleted the adding-user-param-azure branch May 28, 2024 23:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants