Copilot commented on code in PR #7022:
URL: https://github.com/apache/shenyu/pull/7022#discussion_r4037557401
##########
shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/main/java/org/apache/shenyu/plugin/ai/proxy/enhanced/service/AiProxyExecutorService.java:
##########
@@ -62,15 +62,6 @@ public Flux<ChatCompletionChunk> executeDirectStream(final
OpenAiApi mainApi,
final String requestBody, final boolean stream) {
return mainApi.chatCompletionStream(request)
.doOnError(e -> UpstreamErrorLogger.logUpstreamError(LOG, e,
"direct stream"))
- .retryWhen(Retry.max(1)
- .filter(AiProxyExecutorService::isRetryable)
- .onRetryExhaustedThrow((retryBackoffSpec, retrySignal)
-> {
- LOG.warn("Direct stream retry exhausted.
Triggering fallback.",
- retrySignal.failure());
- return new NonTransientAiException(
- "Direct stream failed after 1 retry.
Triggering fallback.",
- retrySignal.failure());
- }))
.onErrorResume(e -> handleDirectFallbackStream(e,
fallbackCtxOpt, requestBody, stream));
Review Comment:
This still invokes the fallback for every stream error, including an error
after one or more chunks have already been emitted. That preserves the reported
`main-partial, fallback-completion` corruption and contradicts #7021's
requirement to fall back only before the first emission. Gate fallback on the
first signal (for example with `switchOnFirst`) so later errors propagate
instead.
##########
shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-proxy/src/main/java/org/apache/shenyu/plugin/ai/proxy/enhanced/service/AiProxyExecutorService.java:
##########
@@ -62,15 +62,6 @@ public Flux<ChatCompletionChunk> executeDirectStream(final
OpenAiApi mainApi,
final String requestBody, final boolean stream) {
return mainApi.chatCompletionStream(request)
.doOnError(e -> UpstreamErrorLogger.logUpstreamError(LOG, e,
"direct stream"))
- .retryWhen(Retry.max(1)
- .filter(AiProxyExecutorService::isRetryable)
- .onRetryExhaustedThrow((retryBackoffSpec, retrySignal)
-> {
- LOG.warn("Direct stream retry exhausted.
Triggering fallback.",
- retrySignal.failure());
- return new NonTransientAiException(
- "Direct stream failed after 1 retry.
Triggering fallback.",
- retrySignal.failure());
- }))
.onErrorResume(e -> handleDirectFallbackStream(e,
fallbackCtxOpt, requestBody, stream));
Review Comment:
Add the regression required by #7021: a main stream that emits a chunk and
then errors while fallback is configured must emit only the main chunk,
propagate the error, and never subscribe to the fallback. The existing service
tests cover only fallback when the main provider fails before its first chunk,
so they cannot detect the mixed-stream behavior this PR targets.
##########
shenyu-admin-listener/shenyu-admin-listener-api/src/main/java/org/apache/shenyu/admin/listener/AbstractNodeDataChangedListener.java:
##########
@@ -174,7 +174,7 @@ public void onSelectorChanged(final List<SelectorData>
changed, final DataEventT
return;
}
SelectorData selectorData =
changed.stream().findFirst().orElseThrow(() -> new
ShenyuException("selectorData is null"));
- final String configKeyPrefix = selectorData.getNamespaceId() +
DefaultNodeConstants.JOIN_POINT + changeData.getSelectorDataId() +
DefaultNodeConstants.JOIN_POINT;
+ final String configKeyPrefix =
StringUtils.defaultString(selectorData.getNamespaceId(),
SYS_DEFAULT_NAMESPACE_ID) + DefaultNodeConstants.JOIN_POINT +
changeData.getSelectorDataId() + DefaultNodeConstants.JOIN_POINT;
Review Comment:
This changes admin-listener key generation for selectors with a null
namespace, but the PR description and linked issue are scoped exclusively to AI
proxy SSE fallback behavior. Please split this unrelated behavioral change into
a separate PR, or explicitly document its motivation and add focused coverage
so it can be reviewed independently.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
