Skip to content

Surface controller services left disabled by schedule_all_controllers#410

Merged
Chaffelson merged 1 commit into
mainfrom
feature/schedule-all-controllers-strict
Jul 9, 2026
Merged

Surface controller services left disabled by schedule_all_controllers#410
Chaffelson merged 1 commit into
mainfrom
feature/schedule-all-controllers-strict

Conversation

@Chaffelson

Copy link
Copy Markdown
Owner

Summary

When enabling, NiFi silently skips controller services that are INVALID (they cannot be enabled). canvas.schedule_all_controllers(..., "ENABLED") waited only for VALID controllers and then returned the activation entity, so the call could report success while leaving one or more services disabled — with no error, warning, or signal to the caller. For automated upgrade/enable flows this makes a real failure state easy to miss.

Change

schedule_all_controllers(pg_id, scheduled, strict=False):

  • After the existing bulk-enable and wait, on the enable path it now re-checks for any controller left in a non-ENABLED state.
  • If any remain: by default it logs a warning naming each (name, id, validation_status); with the new opt-in strict=True it raises ValueError instead — giving automation a deterministic signal.
  • Default behaviour and return type are unchanged (still returns the ActivateControllerServicesEntity). The disable path is untouched.

Tests

tests/test_canvas.py (single-user profile):

  • all-valid enable emits no warning;
  • with an INVALID controller, the default path warns and still returns;
  • with strict=True, it raises ValueError.

Uses the existing StandardSSLContextService fixture to produce a deterministic INVALID controller.

Verified locally: make test NIPYAPI_PROFILE=single-user PYTEST_ARGS="-k schedule_all_controllers" → 3 passed; make lint → flake8 clean, pylint 10.00/10.

When enabling, NiFi silently skips INVALID controller services, so
schedule_all_controllers could report success while leaving services
disabled. It now checks for any controller left in a non-ENABLED state
after an enable and, by default, logs a warning naming them; with the
new opt-in strict=True it raises ValueError instead. Default behaviour
and return type are unchanged, and the disable path is untouched.

Adds tests for the all-valid (no warning), default-warn, and strict-raise
paths using the existing INVALID controller fixture.

@pvillard31 pvillard31 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, backward-compatible fix that turns a previously silent failure into an explicit warning (or an opt-in raise via strict=True), backed by focused tests covering the all-valid, warning, and strict paths.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.97%. Comparing base (ba54425) to head (d0e8291).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #410      +/-   ##
==========================================
+ Coverage   76.93%   76.97%   +0.03%     
==========================================
  Files          41       41              
  Lines        4869     4877       +8     
==========================================
+ Hits         3746     3754       +8     
  Misses       1123     1123              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Chaffelson
Chaffelson merged commit e2785cd into main Jul 9, 2026
12 checks passed
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