Keep Campaign Manager delete report provision check in __init__

[mitre88]

Part of the template-field validation burn-down tracked in #70296.

GoogleCampaignManagerDeleteReportOperator lists both report_name and report_id in template_fields but validates them with truthiness checks in __init__. A Jinja expression is always truthy, so templated values were never actually validated — a pair of expressions rendering to empty strings sailed through and execute silently did nothing. The checks now run at the start of execute() against the rendered values.

Since these are truthiness checks on rendered values (empty string counts as not provided), not pure is None provision checks, they are genuine under the relaxed definition proposed in #70505.

While touching them, the two raise AirflowException usages are narrowed to ValueError per the ongoing exception clean-up; the known_airflow_exceptions.txt entry for this file drops from 2 to 0.

Added a parametrized test that constructs the operator with a templated report_name and validates both failure modes at execute time — it fails against the previous implementation. The class is removed from the exemption list and both prek checks pass locally.

Note: a coordination note for this class was posted on #70296 on Jul 24, but no PR has appeared since; following the "better PR wins" convention rather than letting the entry stall.

---

Was generative AI tooling used to co-author this PR?

• Yes — Claude Code (Fable 5)

Generated-by: Claude Code (Fable 5) following the guidelines

[potiuk]

Thanks — this is the best-behaved instance of the pattern in the batch. Two things it gets right that the sibling PRs don't:

1. It narrows AirflowException to ValueError while moving the raises, and correctly decrements the known_airflow_exceptions.txt entry. That's the direction the project wants, and it's exactly what I've asked for on #70723 where the identical relocation kept AirflowException.
2. The test is parametrized over both failure modes and asserts delete_report.assert_not_called(), so it proves the operator bails before touching the API rather than merely raising somewhere.

Moving the check is right on the merits too: report_name and report_id are template fields, so "provide exactly one of these" was being evaluated against un-rendered Jinja strings — two templated values both looked truthy regardless of what they rendered to.

One test-style suggestion inline.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[potiuk]

Two small things here.

op.execute(context=None) relies on nothing reading the context before the raise — mock.MagicMock() or {} is sturdier if the check ever moves.

More substantively: the test simulates rendering by assigning op.report_name = report_name directly, which tests the guard but not that rendering actually feeds it. #70493 in the same batch does it properly with operator.render_template_fields({"var": {"value": {...}}}) followed by execute — that exercises the real path and would catch a field being dropped from template_fields. Worth borrowing that approach here, since "the value is rendered before the check runs" is the actual contract this PR establishes.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[potiuk]

Retracting my earlier approval — my mistake, and I'm sorry for the churn.

I approved this before reading #70296, which sets out the rule for this burn-down, and by that rule this change moves the wrong kind of check.

A check that only asks whether an argument was passed … belongs in __init__ and must not be moved. Fix these by rewriting in place, not by moving. Use the is not None polarity.

The reasoning in the issue is sound and I'd missed both halves of it:

• With render_template_as_native_obj=True, a field that was provided can render to None, so the same check in execute() reports a supplied argument as missing.
• Raising in the constructor surfaces a static authoring mistake as a Dag import error, rather than once per task instance and per retry on a worker.

Both relocated checks are pure provision checks — they ask which of report_name / report_id was supplied, never what the values are. They collapse neatly into one exactly_one guard in the constructor:

# in __init__ — keep it here
if not exactly_one(report_name is not None, report_id is not None):
    raise ValueError("Please provide exactly one of `report_name` or `report_id`.")

(Import exactly_one from airflow.utils.helpers. #70296 gives this exact construction as the canonical form, and warns against exactly_one(a is None, ...) — the inverted polarity is wrong for three or more arguments.)

Your narrowing of AirflowException to ValueError is right and should be kept.

Good news on mechanics: #70505 (which narrows the hook to allow provision checks written with is not None) merged on 28 July, so the rewrite below passes validate-operators-init and you can still remove the exemption-file entry in this PR — the burn-down goal is unaffected.

There's an active follow-up, #70503, cataloguing already-merged PRs that made exactly this move so they can be put back. Fixing it here saves this PR from joining that list.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[mitre88]

Reworked per the #70296 rule — thanks for the detailed write-up, @potiuk.

The guard is a pure provision check (exactly-one-of report_name/report_id), so it now stays in __init__, rewritten in place with is not None polarity via exactly_one(...), raising ValueError. Nothing moved to execute() (rendered-None already no-ops safely there — no delete call is made). Tests updated to assert failure at construction time. The validate-operators-init hook passes locally on the file and the exemption entry is removed.