sundeep8967 commented on PR #72287: URL: https://github.com/apache/airflow/pull/72287#issuecomment-5752184974
Thanks for the thorough review @potiuk! I have updated the implementation and docs to address all points: 1. **Keyword-based parameter resolution:** In `_run_response_check`, the trigger now inspects `inspect.signature(response_check)` and checks whether `'asset_state_store' in sig.parameters` or `VAR_KEYWORD` (`**kwargs`) is accepted. If accepted, `asset_state_store` is passed by keyword (`asset_state_store=store`), preventing collisions with positional arguments or defaults. Callables taking a single positional argument or keyword-only arguments remain untouched. 2. **Cross-version compatibility:** Guarded `self.asset_state_store` using `store = getattr(self, 'asset_state_store', None)` to prevent `AttributeError` on Airflow versions earlier than 3.3.0 or when running outside an asset watcher context. 3. **Docs & Examples:** Updated `providers/http/docs/triggers.rst` and the `HttpEventTrigger` docstring to document the optional `asset_state_store` parameter and explain that it can be `None`. Updated the example to use the state store rather than the old `Variable.set()` workaround. 4. **Test Suite:** Parametrized `test_run_response_check_callable_shapes` across signature shapes (`single_positional_arg`, `default_second_arg_not_overwritten`, `keyword_only_arg_not_passed_positionally`, `explicit_asset_state_store_kwarg`, `var_keyword_kwargs`) using `mock.create_autospec(AssetStateStoreAccessors, instance=True)`. Also added a test for when `asset_state_store` is not present on the trigger instance. 5. **PR Template & Rebase:** Restored the standard PR template and checklist in the PR body. Rebased onto latest `main` with a clean linear commit history. -- 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]
