ci: Fix Windows CI Errors #7837
Conversation
fpetrini15
force-pushed
the
fpetrini-fix-win-tests
branch
from
a32aa0f
to
e4c039d
Compare
Co-authored-by: Ryan McCormick <[email protected]>
| @@ -85,6 +85,7 @@ if [[ -v WSL_DISTRO_NAME ]] || [[ -v MSYSTEM ]]; then | |||
| BACKEND_DIR=${BACKEND_DIR:=C:/tritonserver/backends} | |||
| SERVER=${SERVER:=/mnt/c/tritonserver/bin/tritonserver.exe} | |||
| export WSLENV=$WSLENV:TRITONSERVER_DELAY_SCHEDULER | |||
| export USE_GRPC=1 | |||
There was a problem hiding this comment.
Yes. It we use HTTP it fails a performance-related test associated with response time. It's been known for several months. Will share more details via DM.
There was a problem hiding this comment.
Can you leave a comment above it for future reference? I think it's too cryptic on it's own to understand why it's there later. If there's some kind of follow-up ticket, you can ref that here too.
qa/L0_trt_plugin/test.sh
Outdated
| @@ -135,7 +135,7 @@ SERVER_LD_PRELOAD=$CUSTOMPLUGIN | |||
| SERVER_ARGS=$SERVER_ARGS_BASE | |||
| SERVER_LOG="./inference_server_$LOG_IDX.log" | |||
|
|
|||
| if [[ ! -v WSL_DISTRO_NAME ]] || [[ ! -v MSYSTEM ]]; then | |||
| if [[ ! -v WSL_DISTRO_NAME ]] && [[ ! -v MSYSTEM ]]; then | |||
There was a problem hiding this comment.
Not familiar with MSYSTEM - but what's the scenario where one is set and not the other?
There was a problem hiding this comment.
Variable WSL_DISTRO_NAME applicable to WSL environment only.
Variable MSYSTEM is applicable for GNU on Windows.
Afraid condition will be always false
There was a problem hiding this comment.
The original intention of this if-condition was to skip the enclosed test on a Windows system. On a Linux system, neither variable will be set, so the enclosed test will be run. On a Windows system, we anticipate that at least one of these will be set, ensuring that we skip the test.
There was a problem hiding this comment.
nit: In that case, can we either:
(a) match the if statement at the top for consistency?
if [[ -v WSL_DISTRO_NAME ]] || [[ -v MSYSTEM ]]; then
echo "Skip this section on Windows"
else
...
fi(b) or set some syntactic sugar naming ex:
IS_WINDOWS=...
if [ IS_WINDOWS ]
# or
if [ ! IS_WINDOWS ]
What does the PR do?
Fixes accumulated errors in Windows CI.
Checklist
<commit_type>: <Title>Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
CI Pipeline ID:
20833929