Fix Airflow operation processing for number data types #2815
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In testing #2799, I came across this issue where number-type component parameter values are being rendered by the DAG as follows because these types were being skipped during processing:
After these changes, the same property appears as it should:
This is not an issue for KFP.
What changes were proposed in this pull request?
A simple 2-line change takes care of handling number-type parameter values (which was erroneously being skipped prior to this). The bulk of the changes here are test-related (see below).
How was this pull request tested?
Previously, no test existed that checked for how number-type values were being processed by Airflow. A case for that is added here to an existing test (
test_create_file_custom_components
). To support this test, I've added a new catalog instance (AIRFLOW_TEST_OPERATOR_CATALOG
) toconftest.py
that loads theTestOperator
defined intests/pipeline/resources/components/airflow_test_operator.py
, which includes a component parameter for each supported data type. I've also updated the sample pipeline used during this test to include theTestOperator
instead of theBashOperator
.Related, I used the same strategy to update
test_same_name_operator_in_pipeline
(also intest_airflow_processor.py
). This test now uses theAIRFLOW_TEST_OPERATOR_CATALOG
that includes theTestOperator
and the test pipeline resource is similarly updated to useTestOperator
parameters.While not strictly necessary, I think with only a few more similar changes, we can completely remove theBashOperator
from our test resources.I've now removed the remaining references to the
BashOperator
in the following tests:test_parse_airflow_component_url
intest_component_parser_airflow.py]
now fetches and parses theTestOperator
fileBashOperator
in the validation pipelineaa_invalid_node_op.pipeline
have been replaced withTestOperator
references (the values and operator-specific details are never interrogated)test_pipeline_aa_parent_node_missing_xcom_push
now uses the newAIRFLOW_TEST_OPERATOR_CATALOG
and references theTestOperator
rather than theBashOperator
Developer's Certificate of Origin 1.1