feat: accept str|enum region/runtime, omit when unset for org default - #57
Merged
Conversation
Widen the region/runtime parameters on WherobotsRunOperator, WherobotsSqlOperator, WherobotsSqlHook, and WherobotsRestAPIHook.create_run to `Optional[Union[str, Region/Runtime]]`. Strings are passed to the API untouched (BYOC regions work without a release); enums are normalized to their value. When region/runtime are omitted they are dropped from the request (region from the /runs query string, runtime from the run payload, and both from the SQL connect() call) so the API applies the organization's configured default. `warn_for_default_region` no longer injects DEFAULT_REGION — it normalizes and returns None when unset. Adds tests for create_run (region omitted / string passthrough) and warn_for_default_region. Note: the SQL path's "omit -> org default" behavior also requires the wherobots-python-dbapi release that makes connect() region/runtime optional; bump the dependency constraint to that version at release time. The run-operator path depends only on the studio-backend API change.
…ntime in prod smoke tests
CI integration tests (which hit prod with the released dbapi) failed:
- test_sql crashed with "'str' object has no attribute 'value'": warn_for_default_region
returned a normalized string, and the released enum-only connect() calls
region.value on it. Normalization belongs at the request boundary
(create_run / connect), so warn now returns the value unchanged
(Optional[Union[str, Region]]); the enum keeps working with the current dbapi
and the new dbapi normalizes internally.
- test_run got a 422 ("runtime field required") because the operator now omits
runtime by default and the (not-yet-deployed) prod API still requires it. The
prod smoke tests are meant to verify a run submits successfully, not the
org-default mechanism (covered by unit tests), so they now pin
runtime=Runtime.TINY for deterministic, transition-robust behavior.
ClayMav
marked this pull request as ready for review
May 28, 2026 00:55
![salty-hambot[bot]](https://avatars.githubusercontent.com/in/2952321?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed by Salty Hambot 🤖🧂
Two real bugs hiding behind if not region / if region_value — empty strings get silently swallowed instead of reaching the API. Quick is None fix in both spots and you're good.
3 finding(s) posted · 1 filtered as false positives.
💬 To request a re-review, comment @salty-hambot review
The prod API returns a TIMED_OUT run status that the provider's RunStatus enum didn't include, so polling a timed-out run raised a Pydantic ValidationError (surfaced by the timeout smoke test once it reaches the real run path). Add TIMED_OUT to RunStatus, treat it as a timeout in Run.is_timeout, and raise the "failed due to timeout" RuntimeError for it in WherobotsRunOperator.execute. Adds a TIMED_OUT case to the execute-state unit test.
sfishel18
approved these changes
May 28, 2026
This was referenced May 28, 2026
ClayMav
added a commit
that referenced
this pull request
May 28, 2026
….0 (#59) PR #57 made region/runtime optional and accept str|enum on the operator side, but the dbapi floor was intentionally left at >=0.26.1 so the PR's CI could resolve before dbapi v0.28.0 was published. v0.28.0 is now on PyPI (released today) and includes the matching client-side change: strings (BYOC regions included) pass through, and an omitted value lets the API apply the org's configured default. This commit raises the floor so we actually depend on the new behavior, refreshes the lock to resolve to 0.28.0, and bumps the package version to 1.7.0 (minor — additive: existing callers passing Region enums still work; existing callers omitting region/runtime now follow the org default instead of the previous hardcoded aws-us-west-2/tiny).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Widens
region/runtimetoOptional[Union[str, Region/Runtime]]acrossWherobotsRunOperator,WherobotsSqlOperator,WherobotsSqlHook, andWherobotsRestAPIHook.create_run..value./runsquery string, runtime from the run payload, and both from the SQLconnect()call), so the API applies the organization's configured default.warn_for_default_regionno longer injectsDEFAULT_REGION— it normalizes the value and returnsNonewhen unset.Why: part of the BYOC region rollout.
Dependencies / release notes
create_run) omits region directly, so it gets org-default behavior as soon as the studio-backend API change ships (wherobots/studio-backend#2208).wherobots.db.connect(), so the "omit → org default" behavior there also needs thewherobots-python-dbapirelease that makesconnect()region/runtime optional (feat: accept str|enum for region/runtime and make them optional wherobots-python-dbapi#67). The dependency constraint (currently>=0.26.1) should be bumped to that release at release time — left unbumped here so this PR's CI can resolve dependencies against the currently published dbapi.Test plan
```
uv run pytest tests/unit_tests/ -> 41 passed
```
New tests:
test_create_run_omits_region_when_none,test_create_run_passes_string_region, andTestWarnForDefaultRegion(None→None, enum→value, string passthrough).