ramitkataria commented on PR #71312:
URL: https://github.com/apache/airflow/pull/71312#issuecomment-5247210199
Thanks both! @kacpermuda @mobuchowski
Yes, I weighed a few alternatives, though I'm new to this code so please
correct me where I've misread it:
- Producer-side gating in the hooks (the #63499 approach). ~32 call sites
across 5 files, and can't go on core `BaseHook` without dropping airflow 2.11.
Filtering at collector read covers every instrumented hook at once.
- Nesting: `{"hook_lineage": {"exclude_datasets": [...]}}`, so
`hook_lineage` takes `bool | dict`. Probably the cleanest answer to
control-vs-scope. I picked siblings thinking it kept the bool path clean, which
was wrong: any non-bool control value widens `dict[str, bool]` in all 8 spots
either way. Same cost, happy to switch.
- Hook-class only (Kacper's earlier hook-scope idea). Doesn't seem to
solve the issue case: the chunk parts and the objects worth keeping come from
the same `S3Hook`, so it drops both. Hence asset patterns.
- `match_mode` for these lists. Built it and then reverted it: two rules
with different lists leave no single mode to record. So always regex,
`match_mode` stays scope-only. This might be a good data point for Kacper's #5.
`hook` as a scope key: doesn't fit as far as I can tell.
`_classify_task_rules` tiers on operator/dag_id/task_id only, one task event
can span several hooks, and resolution returns one `EmissionPolicy`. Hook is
only known per-asset, at collector read. Happy to be wrong here if you see a
way. Note `{"dag_id": x, "task_id": y}` plus `exclude_hook_lineage_hooks`
already gives the dag/task/hook combination you described.
`exclude_datasets` covering operator lineage: I agree and see two options:
- (a) `exclude_datasets` plus one hook dimension, keeps the chunked-upload
case working
- (b) `exclude_datasets` only, hook granularity via `hook_lineage: false`
at task scope
Agree on skipping preprocessors and extra match modes.
Also:
- these lists replace across tiers rather than merge, so a task rule can
narrow or clear (`[]`) a global one. Merging would make a global rule
unescapable locally. Worth confirming that's the behaviour you'd expect.
- filtering runs after `max_assets_per_collector`, so it cuts what's
reported, not what's collected. Anything past the cap was never collected and
no pattern brings it back. Changing that would be producer-side work, so a
bigger change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
