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

fix: Azure 渠道调用DALLE模型报错n_not_within_range #775

Merged
merged 1 commit into from
Dec 17, 2023

Conversation

ShinChven
Copy link
Contributor

  • Add a condition to validate the n value only for non-Azure channels, ensuring it falls within the acceptable range.
  • Fix Azure requestUrl compatibility

我已确认该 PR 已自测通过,相关截图如下:

image

- Add a condition to validate the n value only for non-Azure channels, ensuring it falls within the acceptable range.
- Fix Azure compatibility
@ShinChven
Copy link
Contributor Author

@songquanpeng @simulacraliasing

请帮忙测试变更

@songquanpeng
Copy link
Owner

songquanpeng commented Dec 10, 2023

Hello,可以解释一下这两处变更吗?目前是遇到了哪些 badcase?

@ShinChven
Copy link
Contributor Author

ShinChven commented Dec 11, 2023

Hello,可以解释一下这两处变更吗?目前是遇到了哪些 badcase?

兼容 azure dall-e-3 的 url 和 请求结构。我一次提交的 PR 是正确的,但是在你合并的时候被覆盖掉了这两处,导致无法正确判断是 azure 频道。因此再次提交。

被覆盖应该是由于 a64dc6a 在我的 PR 上做了修改。

@ShinChven
Copy link
Contributor Author

ShinChven commented Dec 14, 2023

Hello,可以解释一下这两处变更吗?目前是遇到了哪些 badcase?

image

OpenAI API 和 Azure OpenAI Service 在请求 dall-e-3 模型时在参数和URL构造上有差异,截图是当前稳定版使用 Azure 渠道时报的错,我提交的两处更改对 Azure 的请求进行了兼容,可以修复这个问题,请考虑合并进去以正确兼容 Azure 渠道。

@ShinChven ShinChven changed the title feat: Add condition to validate n value for non-Azure channels fix: Azure 渠道调用DALLE模型报错n_not_within_range Dec 14, 2023
@@ -102,7 +105,7 @@ func relayImageHelper(c *gin.Context, relayMode int) *OpenAIErrorWithStatusCode
baseURL = c.GetString("base_url")
}
fullRequestURL := getFullRequestURL(baseURL, requestURL, channelType)
if channelType == common.ChannelTypeAzure && relayMode == RelayModeImagesGenerations {
Copy link
Owner

Choose a reason for hiding this comment

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

请问这里为什么要去掉这个条件呢?

Copy link
Contributor Author

@ShinChven ShinChven Dec 17, 2023

Choose a reason for hiding this comment

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

relayMode == RelayModeImagesGenerations
这个判断根本不生效,这个条件是无效的!

channelType == common.ChannelTypeAzure 这一项判断是我在之前完成兼容 Azure 渠道 DALL-E 模型时写的,但我原本PR中根本没有 relayMode == RelayModeImagesGenerations 这个条件。是你合并我的代码时,做了更改,从而导致了根本不会针对 Azure 渠道的请求构建 DALL-E 模型的URL。

Copy link
Owner

Choose a reason for hiding this comment

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

  1. 测试的问题:主要原因是我当前没有立即可用的 Azure DALLE 的渠道可以测试哈(之前有人拉了我进他的组织,但是我一直没有来得及完成设置);
  2. 关于 PR 覆盖的问题这个非常抱歉,我当时合并的时候没有注意到有一个更久的。

@@ -79,7 +79,10 @@ func relayImageHelper(c *gin.Context, relayMode int) *OpenAIErrorWithStatusCode

// Number of generated images validation
if isWithinRange(imageModel, imageRequest.N) == false {
return errorWrapper(errors.New("invalid value of n"), "n_not_within_range", http.StatusBadRequest)
// channel not azure
if channelType != common.ChannelTypeAzure {
Copy link
Owner

Choose a reason for hiding this comment

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

以及这里为什么 Azure 要跳过 check 呢?

Copy link
Owner

Choose a reason for hiding this comment

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

我自己 Azure 用的不多,可能有些地方信息缺失,见谅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

在我的测试中,无论是通过官方的示例代码,还是使用 ChatBox 等客户端调用 DALL-E 模型,这个参数都是空,从而导致进入这个判断以后会被抛出相应的错误。
由于我没有 OpenAI 版的密钥测试,在这里添加一个 Azure 渠道判断以保证 Azure 渠道可以进行正确的请求。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#821

这个PR中解决的是同一个问题。

Copy link
Owner

Choose a reason for hiding this comment

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

在那个 PR 中我已经修改了实现,如果客户端没有传,则重置为 1

@songquanpeng songquanpeng merged commit bc67698 into songquanpeng:main Dec 17, 2023
@songquanpeng
Copy link
Owner

已经合并,thx~

@ShinChven
Copy link
Contributor Author

已经合并,thx~

感谢你合并了我的代码,我 checkout 了 main 分支进行了测试。结果如下:

image

@songquanpeng
Copy link
Owner

我想补充一下为什么会处理这么久,主要原因是我每周只有周日有时间处理,所以如果在合并窗口期没有处理完成,就会导致拖一周

@ShinChven
Copy link
Contributor Author

ShinChven commented Dec 17, 2023

我想补充一下为什么会处理这么久,主要原因是我每周只有周日有时间处理,所以如果在合并窗口期没有处理完成,就会导致拖一周

每次我的代码被人覆盖的时候,我就容易情绪激动。

如果是自己写出来的错误,那我会感到十分羞愧,但这次的情况是被别人的代码覆盖出了一个错误,感觉自己背了一个黑锅,就情绪激动了。

@simulacraliasing
Copy link
Contributor

Hello,可以解释一下这两处变更吗?目前是遇到了哪些 badcase?

兼容 azure dall-e-3 的 url 和 请求结构。我一次提交的 PR 是正确的,但是在你合并的时候被覆盖掉了这两处,导致无法正确判断是 azure 频道。因此再次提交。

被覆盖应该是由于 @simulacraliasing 在我的 PR 上做了修改。

我检查了#754(by @ShinChven), relayMode == RelayModeImagesGenerations 确实在这个PR中。我的PR#764只在#754基础上修改了relay.go中的ImageRequest结构体。我不清楚为什么我的PR会覆盖你的PR,如果可以请告诉我。

@ShinChven
Copy link
Contributor Author

ShinChven commented Dec 20, 2023

Hello,可以解释一下这两处变更吗?目前是遇到了哪些 badcase?

兼容 azure dall-e-3 的 url 和 请求结构。我一次提交的 PR 是正确的,但是在你合并的时候被覆盖掉了这两处,导致无法正确判断是 azure 频道。因此再次提交。
被覆盖应该是由于 @simulacraliasing 在我的 PR 上做了修改。

我检查了#754(by @ShinChven), relayMode == RelayModeImagesGenerations 确实在这个PR中。我的PR#764只在#754基础上修改了relay.go中的ImageRequest结构体。我不清楚为什么我的PR会覆盖你的PR,如果可以请告诉我。

我向你道歉,
这个 PR 是在 merge 的时候被 maintainer modify 了。

@ShinChven
Copy link
Contributor Author

Hello,可以解释一下这两处变更吗?目前是遇到了哪些 badcase?

兼容 azure dall-e-3 的 url 和 请求结构。我一次提交的 PR 是正确的,但是在你合并的时候被覆盖掉了这两处,导致无法正确判断是 azure 频道。因此再次提交。
被覆盖应该是由于 @simulacraliasing 在我的 PR 上做了修改。

我检查了#754(by @ShinChven), relayMode == RelayModeImagesGenerations 确实在这个PR中。我的PR#764只在#754基础上修改了relay.go中的ImageRequest结构体。我不清楚为什么我的PR会覆盖你的PR,如果可以请告诉我。

我之前在 GitHub 和 VSCode 里面翻了好久,都没找到为什么我的 merge request 会跟提交的不一致。
刚才又重新找了一下,代码变动在这里 a64dc6a 我向你道歉。

H0llyW00dzZ pushed a commit to H0llyW00dzZ/one-api that referenced this pull request Dec 20, 2023
…uanpeng#775)

- Add a condition to validate the n value only for non-Azure channels, ensuring it falls within the acceptable range.
- Fix Azure compatibility
H0llyW00dzZ added a commit to H0llyW00dzZ/one-api that referenced this pull request Dec 20, 2023
* feat: add Google Gemini Pro support (songquanpeng#826)

* fest: Add Google Gemini Pro, fix songquanpeng#810

* fest: Add tooling to Gemini; Add OpenAI-like system prompt to Gemini

* refactor: removing unused if statement

* fest: Add dummy model message for system message in gemini model

* chore: update implementation

---------

Co-authored-by: JustSong <songquanpeng@foxmail.com>

* fix: fix Gemini stream problem

* fix: fix xunfei panic error (close songquanpeng#820)

* fix: try to return a more meaningful error message (close songquanpeng#817)

* feat: reset image num to 1 when not given (songquanpeng#821)

* Update relay-image.go

* fix: reset image num to 1 when not given

---------

Co-authored-by: JustSong <songquanpeng@foxmail.com>

* feat: able to set sqlite busy_timeout (songquanpeng#818)

* add sqlite busy_timeout=3000

* chore: update impl

---------

Co-authored-by: JustSong <songquanpeng@foxmail.com>

* feat: update ali relay implementation (songquanpeng#830)

* 修改通译千问最新接口:1.删除history参数,改用官方推荐的messages参数 2.整理messages参数顺序,补充必要上下文信息 3.用autogen调试测试通过

* chore: update impl

---------

Co-authored-by: JustSong <songquanpeng@foxmail.com>

* feat: add condition to validate n value for non-Azure channels (songquanpeng#775)

- Add a condition to validate the n value only for non-Azure channels, ensuring it falls within the acceptable range.
- Fix Azure compatibility

* docs: update readme

* docs: update readme

* fix: fix gemini panic (close songquanpeng#833)

* fix: fix max_tokens check

---------

Co-authored-by: David Zhuang <i@dz.ax>
Co-authored-by: JustSong <songquanpeng@foxmail.com>
Co-authored-by: Ghostz <137054651+ye4293@users.noreply.github.com>
Co-authored-by: Calcium-Ion <61247483+Calcium-Ion@users.noreply.github.com>
Co-authored-by: Oliver Lee <ol_l@msn.cn>
Co-authored-by: ShinChven ✨ <shinchven@gmail.com>
dirname pushed a commit to dirname/one-api that referenced this pull request Dec 22, 2023
…uanpeng#775)

- Add a condition to validate the n value only for non-Azure channels, ensuring it falls within the acceptable range.
- Fix Azure compatibility
@songquanpeng
Copy link
Owner

不好意思,确实我忘记在修改实现后及时同步信息了

greeeds pushed a commit to greeeds/one-api that referenced this pull request Mar 3, 2024
…uanpeng#775)

- Add a condition to validate the n value only for non-Azure channels, ensuring it falls within the acceptable range.
- Fix Azure compatibility
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants