Skip to content

Conversation

@meiji163
Copy link
Contributor

@meiji163 meiji163 commented Aug 20, 2025

Currently if freno finds no backend hosts to collect metrics from it returns 500. We find it preferable to fail open in this case and return 200. For example if hosts are unavailable from one "domain" we can still operate with the metrics from the other domains.

Copilot AI review requested due to automatic review settings August 20, 2025 17:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies the throttler's behavior to return HTTP 200 status code when no hosts are found, rather than treating it as an error condition.

  • Export the NoHostsError variable to make it publicly accessible for error checking
  • Add specific handling for NoHostsError in the throttler check logic to return HTTP 200
  • Add comprehensive test coverage for the throttler check functionality including the new no-hosts behavior

Reviewed Changes

Copilot reviewed 6 out of 30 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/base/throttle_metric.go Export NoHostsError by capitalizing the variable name
pkg/throttle/check.go Add error handling to return HTTP 200 when no hosts are found
pkg/throttle/throttler_test.go Add comprehensive test suite for throttler check behavior
pkg/throttle/check_result.go Reorder imports for consistency
pkg/mysql/mysql_inventory.go Add blank line for import formatting
go.mod Add testify dependency for testing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@meiji163 meiji163 merged commit 4363f9d into master Aug 22, 2025
3 checks passed
@meiji163 meiji163 deleted the meiji163/no-hosts-no-err branch August 22, 2025 00:28
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.

3 participants