damavis / airflow-hop-plugin

Apache Hop plugin for Apache Airflow - Orquestate Apache Hop pipelines and workflows from Airflow
Apache License 2.0
11 stars 6 forks source link

Fix correct param passing from Airflow (version >2.3.3) to HOP #38

Closed vegaskyo closed 5 months ago

vegaskyo commented 5 months ago

Description Issue There was an issue with passing parameters and macro variables from Apache Airflow to Apache HOP. The parameters were not being correctly interpreted in the HOP logs, showing the raw macro variable names (e.g., {{ ds }}) instead of their expected values (e.g., 20-05-2024).

Solution Changed the parameter name from param to hop_param. This adjustment ensures compatibility and correct macro convertion in HOP logs.

Additional Information In Apache Airflow 2.3.3, the params variable is utilized during DAG serialization. Using params as a variable name in third-party operators can lead to issues. For more information, refer to the Apache Airflow Pull Request #20640

vegaskyo commented 5 months ago

You should correct template_fields on both operators so the airflow templates render correctly into their respective values.

Additional info: https://airflow.apache.org/docs/apache-airflow/stable/howto/custom-operator.html#templating

I tested the code without changing template_fields, and all tests passed with both macros and normal string params. Therefore, I believe we do not need to modify template_fields. As long as there is no parameter variable named ‘params’.

PereAL7 commented 5 months ago

You are right. I didn't notice that you only changed the constructor parameter and that is not affected by template_fields. Thanks for your contribution!