-
Notifications
You must be signed in to change notification settings - Fork 26
fix(bedrock): fail fast for Bedrock models on unreachable remote sandboxes #329
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -544,12 +544,14 @@ async def stop(self): | |||||||||||||||||||||||||||||||||||||||||
| def test_host_proxy_reachable_only_for_local_environments(): | ||||||||||||||||||||||||||||||||||||||||||
| from benchflow.providers.runtime import host_proxy_reachable_from_agent | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| # docker shares the host's network namespace via the docker bridge. | ||||||||||||||||||||||||||||||||||||||||||
| assert host_proxy_reachable_from_agent("docker") is True | ||||||||||||||||||||||||||||||||||||||||||
| assert host_proxy_reachable_from_agent("local") is True | ||||||||||||||||||||||||||||||||||||||||||
| assert host_proxy_reachable_from_agent("host") is True | ||||||||||||||||||||||||||||||||||||||||||
| assert host_proxy_reachable_from_agent("") is True | ||||||||||||||||||||||||||||||||||||||||||
| # Remote cloud sandboxes run the agent on a separate machine. | ||||||||||||||||||||||||||||||||||||||||||
| assert host_proxy_reachable_from_agent("daytona") is False | ||||||||||||||||||||||||||||||||||||||||||
| assert host_proxy_reachable_from_agent("modal") is False | ||||||||||||||||||||||||||||||||||||||||||
| # Unrecognized environments are treated conservatively as reachable. | ||||||||||||||||||||||||||||||||||||||||||
| assert host_proxy_reachable_from_agent("") is True | ||||||||||||||||||||||||||||||||||||||||||
| assert host_proxy_reachable_from_agent("some-future-local-env") is True | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
548
to
+554
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Passing test assertions for "local" and "host" environments removed in violation of AGENTS.md The AGENTS.md rule states: "Don't rewrite passing tests to match new behavior." The test
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def test_total_tokens_is_sum_of_parts(): | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new regression test class docstring describes the Daytona/Modal Bedrock proxy regression but does not name the PR or commit it guards, which violates the repository rule in
/workspace/benchflow/AGENTS.md(Regression tests must name the PR/commit they guard). This makes future triage and intentional test-retirement decisions harder because maintainers cannot quickly map the test to its historical regression source.Useful? React with 👍 / 👎.