Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions .github/find-api-model-versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@
def _manifest_versions(bundle: dict) -> dict[str, str]:
packages = bundle.get("packages")
if not isinstance(packages, dict):
raise RuntimeError("PolicyEngine bundle manifest has no package mapping.")
msg = "PolicyEngine bundle manifest has no package mapping."
raise TypeError(msg)

versions = {}
for package_name in REQUIRED_PACKAGES:
package = packages.get(package_name)
if not isinstance(package, dict) or not package.get("version"):
raise RuntimeError(
f"PolicyEngine bundle manifest has no version for {package_name}."
)
msg = f"PolicyEngine bundle manifest has no version for {package_name}."
raise RuntimeError(msg)
versions[package_name] = str(package["version"])
return versions

Expand All @@ -47,26 +47,27 @@ def _data_release_version(bundle: dict, country_id: str) -> str:
data_releases = bundle.get("data_releases")
release = data_releases.get(country_id) if isinstance(data_releases, dict) else None
if not isinstance(release, dict) or not release.get("version"):
raise RuntimeError(
f"PolicyEngine bundle manifest has no {country_id.upper()} data release."
)
msg = f"PolicyEngine bundle manifest has no {country_id.upper()} data release."
raise RuntimeError(msg)
return str(release["version"])


def find_api_model_versions() -> dict[str, str]:
"""Find and validate the package and data versions in the installed bundle."""
bundle = get_current_bundle()
if not isinstance(bundle, dict):
raise RuntimeError("Installed PolicyEngine bundle manifest is not an object.")
msg = "Installed PolicyEngine bundle manifest is not an object."
raise TypeError(msg)

manifest_versions = _manifest_versions(bundle)
for package_name, manifest_version in manifest_versions.items():
installed_version = distribution_version(package_name)
if installed_version != manifest_version:
raise RuntimeError(
msg = (
f"Installed {package_name} version {installed_version} does not match "
f"bundle manifest version {manifest_version}."
)
raise RuntimeError(msg)

expected_constants = {
"policyengine": POLICYENGINE_VERSION,
Expand All @@ -76,10 +77,11 @@ def find_api_model_versions() -> dict[str, str]:
}
for package_name, constant_version in expected_constants.items():
if constant_version != manifest_versions[package_name]:
raise RuntimeError(
msg = (
f"API version for {package_name} is {constant_version}, but the bundle "
f"manifest specifies {manifest_versions[package_name]}."
)
raise RuntimeError(msg)

return {
"POLICYENGINE_VERSION": manifest_versions["policyengine"],
Expand Down
17 changes: 10 additions & 7 deletions .github/scripts/verify_observability_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class VerificationProbe(NamedTuple):
def _authority(endpoint: str) -> str:
parsed = urlparse(endpoint)
if parsed.scheme != "https" or not parsed.hostname:
raise ValueError("collector endpoint must be an HTTPS URL")
msg = "collector endpoint must be an HTTPS URL"
raise ValueError(msg)
return f"{parsed.hostname}:{parsed.port or 443}"


Expand Down Expand Up @@ -193,7 +194,8 @@ def _raise_for_partial_rejection(
rejected = getattr(partial, rejected_field, 0)
if rejected:
message = getattr(partial, "error_message", "")
raise RuntimeError(f"collector rejected {rejected} {signal}: {message}")
msg = f"collector rejected {rejected} {signal}: {message}"
raise RuntimeError(msg)


def _get_json(url: str, access_token: str) -> dict[str, object] | None:
Expand All @@ -208,9 +210,8 @@ def _get_json(url: str, access_token: str) -> dict[str, object] | None:
if error.code == 404:
return None
detail = error.read().decode("utf-8", errors="replace")
raise RuntimeError(
f"Google Cloud verification request failed ({error.code}): {detail}"
) from error
msg = f"Google Cloud verification request failed ({error.code}): {detail}"
raise RuntimeError(msg) from error


def _trace_available(
Expand Down Expand Up @@ -302,7 +303,8 @@ def _wait_for_delivery(
)
if not found
)
raise RuntimeError(f"collector verification data was not stored: {missing}")
msg = f"collector verification data was not stored: {missing}"
raise RuntimeError(msg)
time.sleep(min(poll_seconds, remaining))


Expand Down Expand Up @@ -357,7 +359,8 @@ def verify(
if error.code() is not grpc.StatusCode.UNIMPLEMENTED:
raise
else:
raise RuntimeError("collector unexpectedly accepted application logs")
msg = "collector unexpectedly accepted application logs"
raise RuntimeError(msg)
_wait_for_delivery(
project_id=project_id,
trace_id=trace_id,
Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`.
- `api.quality.environment-variables-load`: Required environment variables and credentials load in a deployed-like environment. (blocking: CI job `test_env_vars`)
- `api.quality.mypy-v2`: Shared query models, FastAPI dependencies, and v2 routes and services pass mypy with untyped definitions disallowed. (blocking: mypy)
- `api.quality.ruff-format`: All Python files are formatted with ruff format. (blocking: ruff format)
- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, UP, W))
- `api.quality.ruff-lint`: All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. (blocking: ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W))
- `api.quality.runtime-evaluated-models-resolve`: Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. (blocking: pytest `tests/unit/test_runtime_evaluated_annotations.py`)
- `api.quality.tests-pass`: The unit, contract and to-refactor test suites pass. (blocking: CI job `test`)
- `api.quality.tests-ruff-exemptions`: Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. (blocking: ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**`)
Expand Down
1 change: 1 addition & 0 deletions changelog.d/ruff-exceptions.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Enabled ruff's exception rules (TRY, EM, BLE): exception messages are built before the raise, raises inside try blocks moved into helpers, the v1 services raise typed exceptions, and every deliberate catch-all carries a reasoned suppression; error responses and message text are unchanged.
8 changes: 4 additions & 4 deletions docs/generated/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ flowchart LR
| `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — |
| `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, and v2 routes and services pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — |
| `api.quality.ruff-format` | All Python files are formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — |
| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, UP, W) | `pr.yml:lint` | — |
| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W) | `pr.yml:lint` | — |
| `api.quality.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** |
| `api.quality.tests-pass` | The unit, contract and to-refactor test suites pass. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — |
| `api.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert and need no docstrings, annotations or named constants for magic numbers (S101, D1, ANN, PLR2004); every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, PLR2004, S101 for `**/tests/**` | `pr.yml:lint` | — |
Expand All @@ -289,7 +289,7 @@ flowchart LR
e_ruff_format["ruff format"]
j_CI_pr_yml_lint["CI pr.yml:lint"]
r_api_quality_ruff_lint("api.quality.ruff-lint (enforced)")
e_ruff_check__ANN__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_["ruff check (ANN, D, E4, E7, E9, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, UP, W)"]
e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_["ruff check (ANN, BLE, D, E4, E7, E9, EM, F, FLY, FURB, I, PIE, PYI, RET, RSE, TC, TRY, UP, W)"]
r_api_quality_runtime_evaluated_models_resolve("api.quality.runtime-evaluated-models-resolve (enforced)")
e_pytest_tests_unit_test_runtime_evaluated_annotations_py["pytest tests/unit/test_runtime_evaluated_annotations.py"]
j_CI_pr_yml_test["CI pr.yml:test"]
Expand All @@ -305,8 +305,8 @@ flowchart LR
e_mypy --> j_CI_pr_yml_typecheck_v2
r_api_quality_ruff_format --> e_ruff_format
e_ruff_format --> j_CI_pr_yml_lint
r_api_quality_ruff_lint --> e_ruff_check__ANN__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_
e_ruff_check__ANN__D__E4__E7__E9__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__UP__W_ --> j_CI_pr_yml_lint
r_api_quality_ruff_lint --> e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_
e_ruff_check__ANN__BLE__D__E4__E7__E9__EM__F__FLY__FURB__I__PIE__PYI__RET__RSE__TC__TRY__UP__W_ --> j_CI_pr_yml_lint
r_api_quality_runtime_evaluated_models_resolve --> e_pytest_tests_unit_test_runtime_evaluated_annotations_py
e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test
r_api_quality_tests_pass --> e_CI_job_test
Expand Down
9 changes: 6 additions & 3 deletions migrations/v1/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,21 @@
provided_connection = config.attributes.get("connection")
if provided_connection is not None:
if provided_connection.dialect.name != "mysql":
raise RuntimeError("Alembic migrations must target a MySQL database")
msg = "Alembic migrations must target a MySQL database"
raise RuntimeError(msg)
else:
database_url = os.environ.get("ALEMBIC_DATABASE_URL") or config.get_main_option(
"sqlalchemy.url"
)
if not database_url:
raise RuntimeError(
msg = (
"ALEMBIC_DATABASE_URL is required; the local SQLite cache is not an "
"Alembic migration target"
)
raise RuntimeError(msg)
if make_url(database_url).get_backend_name() != "mysql":
raise RuntimeError("Alembic migrations must target a MySQL database")
msg = "Alembic migrations must target a MySQL database"
raise RuntimeError(msg)
config.set_main_option("sqlalchemy.url", database_url.replace("%", "%%"))

target_metadata = V1Base.metadata
Expand Down
3 changes: 2 additions & 1 deletion migrations/v2/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ def _configure(connection: Connection) -> None:

def run_migrations_offline() -> None:
"""Raise, because v2 migrations need a live connection to qualify the target."""
raise RuntimeError(
msg = (
"v2 migrations require an online connection so target identity and "
"generated application-data before/after states can be verified"
)
raise RuntimeError(msg)


def run_migrations_online() -> None:
Expand Down
16 changes: 8 additions & 8 deletions policyengine_api/asgi_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,13 @@ async def add_request_context_and_migration_logging(
request.state.policyengine_request_id = request_id
_asgi_request_id.reset(context_token)
context_token = _asgi_request_id.set(request_id)
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass
try:
request_runtime.set_context(
request_id=request_id,
)
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass

def log_native_route(status_code: int) -> None:
Expand All @@ -233,7 +233,7 @@ def log_native_route(status_code: int) -> None:
),
route_impl=RouteImplementation.FASTAPI_NATIVE,
)
except Exception:
except Exception: # noqa: BLE001 # migration logging must never fail the request
pass

def finish_native_route(
Expand All @@ -245,18 +245,18 @@ def finish_native_route(
resolved_route = getattr(request.scope.get("route"), "path", initial_route)
try:
request_runtime.update_request_route(resolved_route)
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass
try:
request_runtime.update_request_status(status_code)
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass
try:
request_runtime.end_request(
status_code=status_code,
error=error,
)
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass

try:
Expand All @@ -280,13 +280,13 @@ def finish_native_route(
request_runtime.set_context(
observability_id=response_observability_id
)
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass
if native_request:
try:
for name, value in request_runtime.response_headers().items():
response.headers[name] = value
except Exception:
except Exception: # noqa: BLE001 # telemetry must never fail the request
pass
_apply_request_id_header(response, request_id)
if response_observability_id is not None:
Expand Down
15 changes: 8 additions & 7 deletions policyengine_api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,14 @@ def _load_policyengine_bundle() -> dict | None:
with open(get_py_manifest()) as manifest_file:
manifest = json.load(manifest_file)
except Exception as exc:
raise RuntimeError(
msg = (
"Could not read PolicyEngine .py bundle manifest from the installed "
"policyengine wheel."
) from exc
)
raise RuntimeError(msg) from exc
if not isinstance(manifest, dict):
raise RuntimeError("PolicyEngine .py bundle manifest must be a JSON object.")
msg = "PolicyEngine .py bundle manifest must be a JSON object."
raise RuntimeError(msg) # noqa: TRY004 # tests expect RuntimeError for a non-object manifest
return manifest


Expand Down Expand Up @@ -162,7 +164,7 @@ def get_bundle_default_dataset_option(country_id: str) -> dict:
country: _resolve_distribution_version(_dist_versions, package_name)
for country, package_name in zip(COUNTRIES, COUNTRY_PACKAGE_NAMES)
}
except Exception:
except Exception: # noqa: BLE001 # unreadable package metadata falls back to 0.0.0; import must not fail
_dist_versions = {}
COUNTRY_PACKAGE_VERSIONS = {country: "0.0.0" for country in COUNTRIES}

Expand Down Expand Up @@ -204,9 +206,8 @@ def _build_runtime_cache_version(
prefix = "e" if scope == "economy_impact" else "r"
digest_length = 10 - len(prefix) - len(schema_version)
if digest_length < 4:
raise ValueError(
f"Runtime cache version for {scope} does not fit in VARCHAR(10)"
)
msg = f"Runtime cache version for {scope} does not fit in VARCHAR(10)"
raise ValueError(msg)

raw = "|".join(
(
Expand Down
2 changes: 1 addition & 1 deletion policyengine_api/country.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ def _normalize_reform_values(
node_type = float
try:
value = float(value)
except Exception:
except (TypeError, ValueError, OverflowError):
pass
normalized_reform[parameter_name][time_period] = node_type(value)
return normalized_reform
Expand Down
11 changes: 6 additions & 5 deletions policyengine_api/data/places.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ def parse_place_code(place_code: str) -> tuple[str, str]:

"""
if "-" not in place_code:
raise ValueError(
msg = (
f"Invalid place format: '{place_code}'. "
"Expected format: STATE_ABBREV-PLACE_FIPS (e.g., NJ-57000)"
)
raise ValueError(msg)
return place_code.split("-", 1)


Expand All @@ -41,9 +42,9 @@ def validate_place_code(place_code: str) -> None:
state_abbrev, place_fips = parse_place_code(place_code)

if state_abbrev.lower() not in get_valid_state_codes():
raise ValueError(f"Invalid state in place code: '{state_abbrev}'")
msg = f"Invalid state in place code: '{state_abbrev}'"
raise ValueError(msg)

if not place_fips.isdigit() or len(place_fips) != 5:
raise ValueError(
f"Invalid FIPS code in place: '{place_fips}'. Expected 5-digit FIPS code"
)
msg = f"Invalid FIPS code in place: '{place_fips}'. Expected 5-digit FIPS code"
raise ValueError(msg)
Loading
Loading