Repository navigation
feat: add set-schedule-active endpoint - admin_data_tools - #1060
Conversation
`set_deployment_schedule_active` mapped every HTTP 404 to "unknown flow". Two very different things return 404: - Django's HTML 404, when `/admin-tools/set-schedule-active/` is not deployed — which is every backend until basedosdados/backend#1060 merges. - The view's own JSON 404, when there is genuinely no DisabledFlowSchedule row. Conflating them is actively misleading. A missing *route* reported as a missing *row* reads as "this armed pipeline has no stored state and the next backend sync will re-pause it" — an alarming conclusion drawn from a backend that simply has not shipped the endpoint yet. I made exactly that misdiagnosis against a live pipeline. Now parses the body first and reports the undeployed case explicitly, noting that it says nothing about whether the flow is armed — `paused` on the Prefect deployment is the answer to that. Also corrects the `flow_name` docstring. It is Prefect's bare deployment name (`au_rba_statistical_tables_flow`); `SyncDeploymentsView` stores `dep["name"]` straight from `/deployments/filter`, not the '<flow>/<deployment>' display form the deploy logs show. The old wording would have sent callers to a 404.
Arming a flow schedule is currently only possible by ticking `is_schedule_active` in the Django admin. There is no programmatic path, so tooling (the databasis MCP, scripts) cannot arm or disarm a pipeline. Adds `POST /admin-tools/set-schedule-active/`, the API equivalent of that tick. It performs the same three steps as `DisabledFlowScheduleAdmin.save_model`: updates the stored flag, stamps `reactivated_at`, and pauses or unpauses the deployment in Prefect 3. All three are necessary. `SyncDeploymentsView` re-enforces the stored `is_schedule_active` state for known deployments, and CI runs it on every merge to main — so a caller that only unpaused via the Prefect API would appear to have armed the flow and then have it silently re-paused by the next unrelated merge. Exposing this as one endpoint keeps the three writes together. Details: - Bearer auth via the existing `_check_bearer_token`, same as the sibling endpoints; rejected before any write. - Prefect is called before the database, mirroring the admin form, so a Prefect failure leaves the stored state untouched rather than recording a change that never reached the scheduler. - Setting the state a flow is already in is a no-op returning `action="no_change"` without calling Prefect — mirrors the admin form, which only acts when the field changes, and gives callers a safe way to read state. - Unknown flow returns 404 pointing at sync-deployments, which is what registers deployments in the first place. Adds tests to the app, which had none: arming, disarming, the no-op, Prefect failure leaving state untouched, unknown flow, malformed payloads, bad JSON, and an invalid token being rejected before any write. 8 tests, all passing.
1333252 to
52f6494
Compare
# Conflicts: # backend/apps/admin_data_tools/urls.py
main renomeou backend/apps/admin_data_tools/views.py pra flow_monitoring.py (e dividiu em bigquery_sync.py/column_import.py) enquanto esta branch estava aberta. Os testes de SetScheduleActiveView ainda faziam @patch("...views.Prefect3Client") — quebraria com ModuleNotFoundError, já que o módulo não existe mais com esse nome.
Winzen
left a comment
There was a problem hiding this comment.
Conflito resolvido
main tinha dividido admin_data_tools/views.py em bigquery_sync.py/column_import.py/flow_monitoring.py enquanto este PR estava aberto. O merge do git detectou o rename e aplicou a SetScheduleActiveView corretamente dentro de flow_monitoring.py — só restou resolver manualmente o urls.py (imports + ordem das rotas). Push em bc14d38.
Bug achado durante a resolução (corrigido)
Os testes faziam @patch("backend.apps.admin_data_tools.views.Prefect3Client") — quebraria com ModuleNotFoundError já que views.py não existe mais com esse nome. Corrigido pra @patch("backend.apps.admin_data_tools.flow_monitoring.Prefect3Client"), que é onde a view de fato importa Prefect3Client agora.
Revisão — pontos de design pra discussão (não bloqueantes)
A lógica central está sólida e bem testada (8 casos, incluindo o de falha do Prefect). Dois pontos que valem conversa:
- O caminho de no-op confia cegamente no banco, sem checar o Prefect.
record.is_schedule_active == desiredretornano_changesem nenhuma chamada ao Prefect — mas se os dois já estiverem dessincronizados (exatamente o cenário de hoje: vários deployments dopipelines#1932foram reativados via API direta do Prefect, sem passar por este endpoint, então o banco ainda dizis_schedule_active=Falsepra eles), chamarset-schedule-activecom o valor que já "parece" ser o atual não corrige nada, mesmo com o Prefect no estado errado. Pode valer a pena o caminho de no-op confirmar contra o Prefect antes de decidir, ou pelo menos documentar essa limitação explicitamente pros consumidores (databasis MCP incluso). - Falha do Prefect vira um 500 cru, não um JSON de erro. Se
client.set_pausedlançar, a exceção sobe sem tratamento — o estado fica intacto (testado, correto), mas a resposta foge do padrão{"error": ...}usado nos outros casos (400/401/404). Umtry/exceptcom resposta JSON padronizada deixaria o contrato mais previsível pra quem consome.
Fora isso: bearer auth antes de qualquer escrita, ordem Prefect-antes-do-banco bem justificada no docstring, e o design do no-op como "auth probe seguro" é uma boa ideia.
What
Adds
POST /admin-tools/set-schedule-active/— the API equivalent of tickingis_schedule_activein the Django admin.Arming a flow schedule is currently only possible through the admin UI. There is no programmatic path, so tooling (the databasis MCP, scripts, CI) cannot arm or disarm a pipeline.
Why it can't just call Prefect
Arming is three writes, and
DisabledFlowScheduleAdmin.save_modeldoes all three:is_schedule_active = Truereactivated_at = now()Prefect3Client().set_paused(deployment_id, paused=False)SyncDeploymentsViewre-enforces the storedis_schedule_activestate for known deployments, and CI POSTs it on every merge to main. So a caller that only unpaused via the Prefect API would appear to have armed the flow and then have it silently re-paused by the next unrelated merge. Keeping the three writes behind one endpoint is the point of this PR.Design
_check_bearer_token, same as the sibling endpoints, rejected before any write.action="no_change"without calling Prefect. Mirrors the admin form (which only acts when the field changes), gives callers a safe way to read state, and doubles as a harmless auth probe.sync-deployments, which is what registers deployments in the first place.Tests
The app had no tests. Adds 8, covering: arming, disarming, the no-op, a Prefect failure leaving stored state untouched, unknown flow, three malformed payloads, invalid JSON, and a bad token being rejected before any write.
Run locally against sqlite (no poetry env or Docker on this machine) — 8/8 pass. All pre-commit hooks pass.
Consumer
The databasis MCP gains
set_deployment_schedule_activein basedosdados/mcp — it returns a 404 error dict until this ships, so merge this first.