Skip to content

fix status codes on http proxy#51658

Merged
zcin merged 6 commits intomasterfrom
CI-755-sheikh-log
Apr 1, 2025
Merged

fix status codes on http proxy#51658
zcin merged 6 commits intomasterfrom
CI-755-sheikh-log

Conversation

@abrarsheikh
Copy link
Contributor

@abrarsheikh abrarsheikh commented Mar 25, 2025

this change aligns the status codes for timeouts/disconnect between tracing and proxy logs

in the process of working on this PR, I realized that We don't have request_timeout_s for gRPC like we do for HTTP. Currently, gRPC relies on an environment variable for request timeouts. We should deprecate the environment variable and introduce a new attribute in the gRPC options schema to configure request timeouts explicitly. #51761

Signed-off-by: Abrar Sheikh <abrar@anyscale.com>
@abrarsheikh abrarsheikh requested review from edoakes and zcin March 25, 2025 00:41
@abrarsheikh abrarsheikh added the go add ONLY when ready to merge, run all tests label Mar 25, 2025
Copy link
Collaborator

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

believe there are some tests that need to be updated (see test_metrics.py)

@abrarsheikh
Copy link
Contributor Author

believe some tests need to be updated (see test_metrics.py)

There are currently no tests for disconnect and timeout in test_metrics.py, would you like me to include those tests?

@edoakes
Copy link
Collaborator

edoakes commented Mar 25, 2025

believe some tests need to be updated (see test_metrics.py)

There are currently no tests for disconnect and timeout in test_metrics.py, would you like me to include those tests?

Oof! Yes we should add some basic ones

Signed-off-by: Abrar Sheikh <abrar@anyscale.com>
Signed-off-by: Abrar Sheikh <abrar@anyscale.com>
Signed-off-by: Abrar Sheikh <abrar@anyscale.com>
Signed-off-by: Abrar Sheikh <abrar@anyscale.com>
@zcin
Copy link
Contributor

zcin commented Apr 1, 2025

Is this ready to merge @abrarsheikh?

@abrarsheikh
Copy link
Contributor Author

Is this ready to merge @abrarsheikh?

yes

@zcin zcin merged commit 6b6bbca into master Apr 1, 2025
5 checks passed
@zcin zcin deleted the CI-755-sheikh-log branch April 1, 2025 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-backlog go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments