Skip to content

Treat unknown ROBOFLOW_ENVIRONMENT values as prod, accept 'production' - #3093

Closed
imbgar-roboflow wants to merge 1 commit into
mainfrom
feature/strict-roboflow-environment
Closed

imbgar-roboflow wants to merge 1 commit into
mainfrom
feature/strict-roboflow-environment

Conversation

@imbgar-roboflow

Copy link
Copy Markdown
Contributor

What

ROBOFLOW_ENVIRONMENT now accepts prod (or production) and staging. Any other value warns and falls back to prod. Before this change, anything other than prod selected staging.

Applied identically to the three copies of the resolver:

  • inference_sdk/regions.py
  • inference/core/utils/regions.py
  • inference_models/inference_models/configuration.py

An empty ROBOFLOW_ENVIRONMENT is treated as unset, so the legacy PROJECT signal still decides.

Why

The EU production async-serverless consumers set ROBOFLOW_ENVIRONMENT=production. On v1.7.2 (which includes #2701), that value selects staging, so the two defaults they don't override resolve to EU staging:

  • RF_API_BASE_URL → https://api.roboflow-eu.one (inference_sdk WebRTC TURN config and model stats)
  • BUILDER_ORIGIN → https://app.roboflow-eu.one

API_BASE_URL and ROBOFLOW_API_HOST are set explicitly on those pods, so the main API traffic is unaffected. The values are also being corrected to prod in async-serverless. This change makes the resolver safe regardless.

Falling back to prod is also the safer direction for typos. A misspelled production value used to point production traffic at staging without any error. Now a misspelled staging value reaches prod with staging credentials, which fails loudly, and a warning is logged.

Values currently set across the org's deployments: prod, staging, and production in the two EU async-serverless files. None of them changes meaning, except production, which now resolves correctly.

Sibling: roboflow/roboflow-python#513 uses the same rules.

Testing

  • tests/inference_sdk/unit_tests/test_regions.py: 15 passed
  • tests/inference/unit_tests/core/utils/test_regions.py: 16 passed (run against the module in isolation; the module is identical to the SDK copy)
  • inference_models/tests/unit_tests/test_configuration.py: 23 passed
  • tests/inference_cli/unit_tests/lib/test_env.py: 11 passed
  • black 26.3.1 + isort 5.13.2 clean

Any value other than 'prod' used to select staging. EU production
async-serverless consumers set ROBOFLOW_ENVIRONMENT=production, so on
v1.7.2 their RF_API_BASE_URL and BUILDER_ORIGIN defaults resolved to EU
staging hosts. Accept 'production' as prod, keep 'staging', and warn and
fall back to prod for anything else. An empty value falls through to the
legacy PROJECT signal as if unset.
@github-actions

Copy link
Copy Markdown
Contributor

👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely.

🚦 This PR is marked Ready for review, so automated Claude review will run — and every pass spends real credits.

Warning

💸 The Claude reviewer bills in credits, not vibes

Automated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared.

Draft when unsure, Ready when you mean it:

  • 🌱 Not sure the PR is in good shape yet? Keep it (or set it back) as a draft — drafts pause review, so you can push and iterate without burning credits on a moving target.
  • 💪 Feel strong about the contents? Mark it Ready for review and the reviewer will take a look.

However you get there, arrive prepared:

  • 🧱 Bring a SOLID, thorough PR. Point your local agent at our skills/ to tune it to our guidelines first — or, if you are one of those fabled carbon-based contributors, read them yourself. A half-baked diff costs exactly the same to review as a finished one.
  • ✅ Resolve every comment before you re-request review. Re-requesting with threads still open means paying twice for the same conversation.
  • 🔁 Do not use CI review as an inner loop for a local agent. The reviewer is not a step-by-step debugger — do the unfolding locally and arrive with the answer, not the search.
  • 🙋 If something looks off, ask a human. One question to a maintainer is cheaper and faster than three rounds of agent re-review chasing a misread.

Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.

  • Prefer to skip automated review entirely? Add the skip-claude-review label.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Claude review started at commit 3dcf62bcc34a443226bbbd35e0f2ff0bf57a32c9.

New commits are not auto-reviewed. Add the claude-review label to request a re-review — the label is consumed when the review starts, so just add it again next time.

@github-actions

Copy link
Copy Markdown
Contributor

Claude review summary

Skills: review-core-infra, review-sdk, review-inference-models-pkg, review-topic-backward-compat-and-versioning, review-topic-external-contract-and-silent-fallback, review-topic-secure-gateway-url-wrapping, review-topic-test-hygiene

I traced all three resolver copies (inference/core/utils/regions.py, inference_sdk/regions.py, inference_models/inference_models/configuration.py) and the other two ROBOFLOW_ENVIRONMENT readers (inference_cli/lib/enterprise/inference_compiler/constants.py, inference/core/interfaces/webrtc_worker/modal.py — both only ever set prod/staging, so they stay consistent). The logic is correct: production → prod, staging → staging, empty → legacy project/default, unknown → warn + prod. Unit tests exercise each branch and the assertions match the code. No blocking code defects found.

One release item is open (below), so this is not yet a pass.

📋 Changelog required (user-visible behavior change)

This changes the user-visible resolution of ROBOFLOW_ENVIRONMENT in the versioned inference-models package (and the SDK): previously any non-prod value selected staging; now unknown values warn and fall back to prod, empty is treated as unset, and production is accepted. Please add a user-facing entry under ## Unreleased in inference_models/docs/changelog.md describing this. The doc update to environment-variables.md is good, but the changelog entry is still missing.

Maintainers: this requires a release-time version change for inference-models (behavior change to configuration.py). The SDK/core regions.py copies carry the same behavior change.

Minor (non-blocking): in configuration.py, an explicitly empty ROBOFLOW_ENVIRONMENT="" emits a spurious Unknown ROBOFLOW_ENVIRONMENT '' warning before falling back to prod, whereas the regions.py copies treat empty as unset silently. Same end host, only a cosmetic warning difference — mentioning for awareness, not requesting a change.

Re-review is not automatic: add the claude-review label to request another pass.

Reviewed at HEAD: 3dcf62b

@PawelPeczek-Roboflow

Copy link
Copy Markdown
Collaborator

@imbgar-roboflow what we gain by silently fall back to prod on unknown values instead raising error?

@imbgar-roboflow

Copy link
Copy Markdown
Contributor Author

Closing: out of scope for this pass. The EU prod values are being fixed directly in roboflow/async-serverless#461.

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.

2 participants