)]}'
{"specs/2027.1/approved/services-api.rst":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b1ddacbd30a213a9ef7c019e5955a66163d4db9e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"designed for pluggable storage backends, but no OpenStack project"},{"line_number":161,"context_line":"has ever shipped a second backend."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"Nova recognized this and moved to placing SQLAlchemy queries"},{"line_number":164,"context_line":"directly in versioned objects, eliminating the redundant layer."},{"line_number":165,"context_line":"Cyborg is small enough today (~40 DB API methods) that establishing"},{"line_number":166,"context_line":"this direction now is low-cost. Waiting until after services,"}],"source_content_type":"text/x-rst","patch_set":1,"id":"54e20619_7f028720","line":163,"updated":"2026-08-24 03:00:00.000000000","message":"The spec\u0027s key architectural decision — placing SQLAlchemy queries directly in the Service versioned object and setting that as the pattern for all future Cyborg objects — is justified by the claim that \u0027Nova recognized this and moved to placing SQLAlchemy queries directly in versioned objects, eliminating the redundant layer.\u0027 That is not Nova\u0027s practice: Nova retains nova/db/api.py and nova/db/main/api.py as its DB API layer, and nova/objects/*.py (including nova/objects/service.py, which the spec itself cites as a reference) call those db.api functions rather than embedding SQLAlchemy. The spec even cites nova/db/main/api.py lines 574-576 as its reference implementation for last_seen_up, i.e., citing the very layer it says Nova eliminated. Changing Cyborg\u0027s established DB access pattern (CyborgObject -\u003e cyborg/db/api.py -\u003e sqlalchemy/api.py) is a significant, hard-to-reverse direction and should stand on accurate reasoning or real precedent (the closest is Placement\u0027s repository pattern, not objects with embedded SQLAlchemy).\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: The spec sets a precedent for every future Cyborg object based on a premise reviewers can falsify. If merged as-is, the Developer impact section directs contributors away from the project\u0027s existing DB API layer on grounds that do not hold, and reversing course after several objects embed SQLAlchemy would be costly.\n\n**Suggestion**:\nCorrect the justification: state that this is a deliberate Cyborg decision to skip the abstraction (optionally citing the pluggable-backend removal discussion or Placement\u0027s repository approach), drop the false Nova precedent, and note that existing objects are unaffected so the direction can be revisited. Also avoid pinning a reference to mutable upstream line numbers.","commit_id":"70dd3a98df373ba46bf2d94173561fc285d1e966"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b1ddacbd30a213a9ef7c019e5955a66163d4db9e","unresolved":false,"context_lines":[{"line_number":243,"context_line":""},{"line_number":244,"context_line":"Computed at API response time (not stored):"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"* ``state \u003d \"up\"`` if"},{"line_number":247,"context_line":"  ``(now - service.last_seen_up) \u003c service_down_time``"},{"line_number":248,"context_line":"* ``state \u003d \"down\"`` otherwise"},{"line_number":249,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"6fe7152a_798bab54","line":246,"updated":"2026-08-24 03:00:00.000000000","message":"The spec defines the table column last_seen_up as nullable and \u0027set only when report_count increases\u0027, then defines liveness purely as `state \u003d \"up\" if (now - service.last_seen_up) \u003c service_down_time`. For any row where last_seen_up is NULL, this computation is undefined: a newly registered RPC service has no heartbeat yet (report_count defaults to 0 and last_seen_up is NULL until the first _report_state run up to report_interval later), and GET /v2/services/{uuid} deliberately returns WSGI services that never heartbeat. The spec even acknowledges \u0027state is not meaningful\u0027 for WSGI services without defining what value the API returns. Nova avoids this by falling back to updated_at/created_at in service_is_up().\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: An implementer following the spec literally gets a TypeError or an arbitrary state value for freshly registered services (which appear down or error until the first heartbeat, up to report_interval seconds) and for every cyborg-api row fetched via GET /v2/services/{uuid}. Because the response schema is part of a permanent API microversion, an under-specified state field would have to be fixed in another microversion later.\n\n**Suggestion**:\nSpecify the NULL case explicitly in the Liveness detection section: either follow Nova\u0027s fallback (`last_seen_up or updated_at or created_at`) or define that a row with last_seen_up NULL reports state \u003d \"down\" / state omitted, and state exactly what the GET /v2/services/{uuid} response contains for WSGI services (e.g., \"state\": null or the field omitted).","commit_id":"70dd3a98df373ba46bf2d94173561fc285d1e966"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"b1ddacbd30a213a9ef7c019e5955a66163d4db9e","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"Example response::"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    {"},{"line_number":270,"context_line":"        \"services\": ["},{"line_number":271,"context_line":"            {"},{"line_number":272,"context_line":"                \"uuid\": \"d9d2f0c6-3e4a-4b8c-9f1a-2b3c4d5e6f7a\","}],"source_content_type":"text/x-rst","patch_set":1,"id":"aa4db7d7_553bf092","line":269,"updated":"2026-08-24 03:00:00.000000000","message":"The spec states a use case: \u0027As an operator performing a rolling upgrade, I want to see which service version each host is running\u0027, adds a mandatory `version` column to the services table, and justifies registering cyborg-api rows specifically \u0027for version tracking ... important for rolling upgrade version checks\u0027. However, the specified GET /v2/services response schema (the example payload) contains only uuid, host, binary, state, report_count, updated_at, and created_at — no `version` field — and GET /v2/services/{uuid} is described as \u0027returns a single service object\u0027 with no schema that adds it. As specified, the API cannot satisfy its own rolling-upgrade use case, and nothing else in the spec exposes the version to operators.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: If implemented as written, the primary operator benefit claimed for the version groundwork is unreachable through any API, and exposing it later requires another microversion and API-contract churn — costly given that the template requires API changes to be specified carefully upfront because they must be supported forever.\n\n**Suggestion**:\nAdd `version` to the GET /v2/services and GET /v2/services/{uuid} response schemas (or explicitly document that version exposure is deferred and remove/adjust the rolling-upgrade operator use case accordingly).","commit_id":"70dd3a98df373ba46bf2d94173561fc285d1e966"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4b8ffb538ae1813096c9427601d2861a89fcdb8a","unresolved":false,"context_lines":[{"line_number":286,"context_line":""},{"line_number":287,"context_line":"Optional query parameters: ``binary``, ``host``."},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"Example response::"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    {"},{"line_number":292,"context_line":"        \"services\": ["}],"source_content_type":"text/x-rst","patch_set":2,"id":"c2d27316_69610c2e","line":289,"updated":"2026-08-24 03:37:01.000000000","message":"The problem description states operators need to answer \u0027Which hosts are running cyborg-agent and when did they last report in?\u0027 and the use case section repeats the health-visibility goal. However, the GET /v2/services response schema contains only state, report_count, created_at, and updated_at. The last_seen_up timestamp, which the spec carefully introduces as the only reliable liveness datum, is never exposed, so the API can answer up/down but not \u0027when did they last report in\u0027.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: The primary operator question the spec sets out to answer is only half-answered. Operators debugging flapping agents get a boolean state and a monotonically increasing counter but no way to see how stale the heartbeat is (e.g., 55 seconds vs. 5 seconds since last report), which is the actual diagnostic value.\n\n**Suggestion**:\nAdd last_seen_up (e.g., exposed as last_heartbeat or last_seen_up in the response, null-omitted for WSGI services like state) to the GET /v2/services and GET /v2/services/{uuid} response schemas, or explicitly narrow the stated use case wording if the timestamp is intentionally omitted.","commit_id":"e703257517f6a9bd2b26189dba3ba98af6091fc8"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4b8ffb538ae1813096c9427601d2861a89fcdb8a","unresolved":false,"context_lines":[{"line_number":324,"context_line":"UUID; the ``state`` field is omitted from the response since they"},{"line_number":325,"context_line":"do not heartbeat."},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"**DELETE /v2/services/{uuid}** (delete stale service entry)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"Response code: 204"},{"line_number":330,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"aa841522_bfe0e703","line":327,"updated":"2026-08-24 03:37:01.000000000","message":"The spec adds a periodic _report_state() task that increments report_count and calls service.save() every report_interval, and separately adds DELETE /v2/services/{uuid}. It never defines what the running service does when its row has been deleted: save() against a missing row will fail (or silently recreate it depending on implementation), producing either a crash loop in the periodic task or resurrection of the entry the operator just removed. Nova\u0027s equivalent _report_state() handles this explicitly by catching ServiceNotFound and re-registering, plus retry/dead-service handling for transient DB errors; the spec cites Nova\u0027s pattern elsewhere but omits this error path.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: An operator who deletes an entry for a host whose agent is actually alive (misjudged staleness, wrong UUID, or a race during decommissioning) gets undefined behavior: either the entry instantly reappears in GET /v2/services (making DELETE appear broken) or the agent\u0027s periodic task raises every 10 seconds until restart. Both are operator-facing failure modes of the new API.\n\n**Suggestion**:\nSpecify the DELETE/heartbeat interaction: follow Nova\u0027s report_state behavior, i.e. on ServiceNotFound during save(), re-register the service row (and log), and tolerate transient DB errors with a bounded retry. Also state whether DELETE should be rejected (or warned about) for a service whose state is currently \u0027up\u0027, since the stated use case is stale entries for decommissioned hosts.","commit_id":"e703257517f6a9bd2b26189dba3ba98af6091fc8"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"4b8ffb538ae1813096c9427601d2861a89fcdb8a","unresolved":false,"context_lines":[{"line_number":342,"context_line":""},{"line_number":343,"context_line":"::"},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    service_policies \u003d ["},{"line_number":346,"context_line":"        policy.RuleDefault(\u0027cyborg:service:get_all\u0027,"},{"line_number":347,"context_line":"                           \u0027rule:admin_api\u0027),"},{"line_number":348,"context_line":"        policy.RuleDefault(\u0027cyborg:service:get_one\u0027,"}],"source_content_type":"text/x-rst","patch_set":2,"id":"3d5908fd_08f38c29","line":345,"updated":"2026-08-24 03:37:01.000000000","message":"The spec proposes service policies as plain policy.RuleDefault entries with check string \u0027rule:admin_api\u0027 in a rule list. The approved 2026.2 consistent-and-secure-rbac spec migrates Cyborg policy to per-resource modules under cyborg/policies/ using DocumentedRuleDefault with scope_types and deprecated bridges for every endpoint, and removes legacy rule lists from cyborg/common/policy.py. New policies added after that migration should follow the new pattern, or they will re-introduce exactly the legacy shape the RBAC spec eliminates.\n\n**Severity**: WARNING | **Confidence**: 0.85\n\n**Impact**: If implemented as specified, the new cyborg:service:* policies would be the only post-migration policies without scope_types or documented defaults. Operators using enforce_new_defaults would get policies invisible to oslo.policy back-compat tooling, and the services endpoints would diverge from the persona model (e.g., no reader persona decision) applied to the comparable admin-only inventory endpoints.\n\n**Suggestion**:\nUpdate the policy section to define the three rules as DocumentedRuleDefault entries with scope_types in a cyborg/policies/services.py module, with rule:admin_api as the deprecated bridge (matching the hardware-inventory mapping in the RBAC spec), and decide the persona for the new defaults explicitly.","commit_id":"e703257517f6a9bd2b26189dba3ba98af6091fc8"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a8faf6e12b5092a16862e542a0918ad506597320","unresolved":false,"context_lines":[{"line_number":19,"context_line":"cyborg-agent on compute-7 alive?\""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Problem description"},{"line_number":23,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Cyborg runs three service binaries: two RPC services --"}],"source_content_type":"text/x-rst","patch_set":3,"id":"8de92d9e_4fc5dd33","line":22,"updated":"2026-08-24 03:57:34.000000000","message":"The review base commit 6f0e1fb (\u0027Add 2027.1 spec structure\u0027) explicitly changed all 2027.1 template section headers to title case. This spec uses sentence case for the corresponding sections and is even internally inconsistent: \u0027Documentation Impact\u0027 (line 676) is title case while \u0027Problem description\u0027 (line 22), \u0027Proposed change\u0027 (line 67), \u0027Data model impact\u0027 (line 475), \u0027REST API impact\u0027 (line 481), \u0027Security impact\u0027 (line 494) and others are sentence case.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: Cosmetic inconsistency across the 2027.1 spec directory plus mixed casing within a single document, undoing the uniformity the base commit deliberately introduced. No behavior impact; does not block merge.\n\n**Recommendation**:\nRename the sentence-case section headings to the template\u0027s title case (Problem Description, Proposed Change, Data Model Impact, REST API Impact, Security Impact, Notifications Impact, Other End User Impact, Performance Impact, Other Deployer Impact, Developer Impact, Upgrade Impact).","commit_id":"d6cb543371c51dd7bec7daf36d4800d6864e5a70"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a8faf6e12b5092a16862e542a0918ad506597320","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        ]"},{"line_number":314,"context_line":"    }"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"WSGI services (``cyborg-api``) are filtered out of the list"},{"line_number":317,"context_line":"response because they do not heartbeat and ``state`` is not"},{"line_number":318,"context_line":"meaningful for them."},{"line_number":319,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"de716765_72202944","line":316,"updated":"2026-08-24 03:57:34.000000000","message":"The spec filters WSGI services (topic\u003dNone, i.e. cyborg-api) out of GET /v2/services responses because they never heartbeat and would always show state\u003ddown. However, DELETE /v2/services/{uuid} is the spec\u0027s only cleanup mechanism and requires an already-known UUID, and the re-registration safety net only exists for RPC services via the _report_state() periodic task. cyborg-api never heartbeats and its row never re-registers, so once a controller host running cyborg-api is decommissioned, its row is permanently invisible in the list API and cannot be cleaned up through the API this spec introduces.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Use Case 2 is not satisfiable through the API for the service binary most likely to leave orphaned rows (an API service on every controller that never reports and never disappears). Operators must fall back to direct database access, the exact external-tooling dependency the spec argues against. Defect is limited to operational handling of an edge population, so warning severity, not higher.\n\n**Suggestion**:\nState how stale cyborg-api rows are managed: either permit an admin-only binary/host filter that can surface WSGI entries in the list (so their UUIDs are discoverable for DELETE), or document that WSGI rows must be pruned via a documented operational procedure, or defer cyborg-api registration until the version-tracking consumer exists.","commit_id":"d6cb543371c51dd7bec7daf36d4800d6864e5a70"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"a8faf6e12b5092a16862e542a0918ad506597320","unresolved":false,"context_lines":[{"line_number":628,"context_line":"* **Tests**: Unit tests for the ``Service`` object and REST"},{"line_number":629,"context_line":"  controller. Functional/API tests."},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"* **Client updates** (separate repo): New ``openstack accelerator"},{"line_number":632,"context_line":"  service list`` and ``openstack accelerator service show``"},{"line_number":633,"context_line":"  commands in ``python-cyborgclient``."},{"line_number":634,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"400b30f0_6a073087","line":631,"updated":"2026-08-24 03:57:34.000000000","message":"\u0027Other end user impact\u0027 says python-cyborgclient needs three new commands: openstack accelerator service list, service show, and service delete, matching the three endpoints this spec adds. The Work Items client bullet, however, lists only \u0027service list\u0027 and \u0027service show\u0027, dropping the delete command without the explicit deferral note used for \u0027service set\u0027.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: The spec\u0027s only operator-facing cleanup mechanism (deleting stale entries, Use Case 2) loses its CLI work item, so the delete capability could ship API-only or be forgotten during implementation. Low-impact documentation omission with a one-line remedy.\n\n**Recommendation**:\nAdd \u0027openstack accelerator service delete\u0027 to the client-updates work item, or state explicitly that it is deferred alongside \u0027service set\u0027 until disable semantics are defined.","commit_id":"d6cb543371c51dd7bec7daf36d4800d6864e5a70"}]}