will this also check context['triggering_asset_events']?

I think we have two functions here and they are called in different places.
1.) check_context_key_usage_in_call on Expr::Call(call_expr @ ExprCall {func, arguments, ..},)
2.) check_context_key_usage_in_subscript on Expr::Subscript(subscript_expr)

The check_context_key_usage_in_subscript will capture context['triggering_dataset_events'] and raise diagnostic to suggest update to use context['triggering_asset_events']

I'm sorry... my description was totally wrong...

Here's what we want to check

# airflow 2.11

context["inlet_events"]["this://is-url"] # this is not ok in airflow 3+
context["inlet_events"][Dataset("this://is-url")] # this is still ok

We do not support accessing inlet events through a string (used to be the URI of Dataset) now. We can either change it to Asset(uri="this://is-url") or just warn the user to use the Asset object instead of a string.

ah I see. In this case, instead of checking the uri attribute on context["inlet_events"], we actually should check if context["inlet_events"] is being accessed again through subscript, and the key is of type str. If we detect this, we should suggest to change the str (i.e., the url) to Asset(uri="this://is-url")

also, I wanted to double-check that the implementation for external_trigger is still correct.

The new rule added here are:

1. triggering_dataset_events -> triggering_asset_events, use Rename to suggest a fix.
2. context["dag_run"].external_trigger is deprecated, use Message to warn.
3. context["inlet_events"]["this://is-url"] is deprecated. use Message to warn, provide a example to use Asset("this://is-url").

2. and 3. above support detection for .get() as well, or even variable binding inlet_events = context["inlet_events"]; inlet_events["this://is-url"]

How the rules for the context keys are triggered.

1. check_context_key_usage_in_call is used to capture case context.get("key"), triggered on Expr::Call
2. check_context_key_usage_in_subscript is used to capture case context["key"], triggered on Expr::Subscript

In the current implementation, we need to implement the same rule in two places mentioned above. Therefore, I've done a refactor to extract the rules and the report diagnostic into the following functions.

1. check_deprecated_context_key (check for deprecated context key, e.g., context["triggering_dataset_events"])
2. check_deprecated_context_key_value_access (check for deprecated access on context key's value, e.g., context["inlet_events"]["this://is-url"])
3. check_removed_attribute_access_on_context_key (check for deprecated access on attribute on context key's value, e.g., context["dag_run"].external_trigger)

In this way, we can keep our rule and diagnostic reporting in a single place, but calling these functions in the two rule trigger functions above.

I also extracted the common check logic into multiple helper functions.

1. is_context_variable: checks if an expression is a valid Airflow context variable. If should be either an kwarg parameter **context, or a variable from get_current_context().
2. is_context_key_access: checks for key access through subscription context["key"] or a call context.get().
3. is_value_from_context_key: checks if an expression is or resolves to an access of a specific context key. e.g., inlet_events = context["inlet_events"]; inlet_events.
4. is_removed_context_key_attribute: checks if an attribute access on a specific context key; e.g., dagrun = context["dag_run"]; dagrun.external_trigger.

Hi @Lee-W , thank you for caching this!! I just made an update to the message to make it bit more accurate and consistent. Can you have a look again? Thanks.

Looks great :)