Skip to content

feat(dags): add tags to all workflows - #204

Merged
Lee-W merged 5 commits into
masterfrom
add-tags
Jul 28, 2026
Merged

feat(dags): add tags to all workflows#204
Lee-W merged 5 commits into
masterfrom
add-tags

Conversation

@Lee-W

@Lee-W Lee-W commented Jul 17, 2026

Copy link
Copy Markdown
Member

Classify workflows by layer, purpose, and integration. Enforce that every discovered Dag has at least one tag.

Types of changes

  • Bugfix
  • New feature
  • Refactoring
  • Breaking change (any change that would cause existing functionality to not work as expected)
  • Documentation Update
  • Other (please describe)

Description

Checklist

  • Add test cases to all the changes you introduce
  • Run make lint and make test locally to ensure all linter checks and testing pass
  • Update the documentation if necessary

Steps to Test This Pull Request

Expected behavior

Related Issue

Additional context

Classify workflows by layer, purpose, and integration. Enforce that every discovered Dag has at least one tag.

@ttw225 ttw225 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good overall — the layer / purpose / integration split is clean and easy
to follow.

The one thing I'd like to align before merge is the notification tag: five of
the app Dags post to Discord, but only two carry it (details in the inline
comment on finance_bot). Since it's your own taxonomy, unifying those five
keeps the purpose axis consistent — happy to defer if you meant domain to take
priority there.

Optional: if the layer values are meant to be a fixed set (app / ods /
maintenance), a short note in CLAUDE.md would help the next person pick the
right one.

Thanks for putting this together 🙏

Comment thread dags/app/user_profile/dag.py Outdated
Comment thread dags/app/finance_bot/dag.py Outdated
Comment thread tests/test_dag_integrity.py
Comment thread dags/airflow_log_cleanup.py
Lee-W added 4 commits July 25, 2026 16:16
Add the `gemini` integration tag to the user_profile Dag, the only Dag
calling an external AI service (gemini-2.0-flash) and previously the only
one without an integration tag.

Deliberately omits `bigquery`: nearly every Dag touches it, so it carries
no filtering value on the integration axis.
Add the `notification` purpose tag to DISCORD_FINANCE_REMINDER,
proposal_count and registration_statistics, which post to Discord but
previously carried only their domain tag.

Filtering the purpose axis for `notification` now returns all five Dags
that notify people instead of two. Existing domain tags are kept.
Record that every Dag carries exactly one layer tag from the closed set
`app` / `ods` / `maintenance`, so the next person adding a Dag knows which
layer it belongs to.

Domain and integration axes stay open-ended — held by convention and review
rather than a whitelist, which would rot as new integrations arrive.
`bigquery` is deliberately untagged: nearly every Dag touches it.
The module docstring only mentioned the import/parse smoke test, but the
module also asserts Dag ID uniqueness and that every Dag carries tags.

@ttw225 ttw225 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@Lee-W
Lee-W merged commit 3383728 into master Jul 28, 2026
2 checks passed
@Lee-W
Lee-W deleted the add-tags branch July 28, 2026 03:53
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