olegkachur-e commented on code in PR #47993:
URL: https://github.com/apache/airflow/pull/47993#discussion_r2022665956
##########
providers/apache/beam/src/airflow/providers/apache/beam/operators/beam.py:
##########
@@ -37,15 +37,22 @@
from airflow.providers.apache.beam.triggers.beam import
BeamJavaPipelineTrigger, BeamPythonPipelineTrigger
from airflow.providers.google.cloud.hooks.dataflow import (
DataflowHook,
- DataflowJobStatus,
process_line_and_extract_dataflow_job_id_callback,
)
from airflow.providers.google.cloud.hooks.gcs import GCSHook, _parse_gcs_url
from airflow.providers.google.cloud.links.dataflow import DataflowJobLink
from airflow.providers.google.cloud.operators.dataflow import CheckJobRunning,
DataflowConfiguration
-from airflow.providers.google.cloud.triggers.dataflow import (
- DataflowJobStatusTrigger,
-)
+
+try:
+ from airflow.providers.google.cloud.triggers.dataflow import
DataflowJobStateCompleteTrigger
+except ImportError:
+ from airflow.exceptions import AirflowOptionalProviderFeatureException
+
+ raise AirflowOptionalProviderFeatureException(
+ "Failed to import DataflowJobStateCompleteTrigger. To use this
functionality, please install"
+ "the apache-airflow-providers-google >= 15.0.0"
+ )
Review Comment:
Yes, my idea was: this change will be included in some new version of Beam
provider, so people who want to use it will get known to use a fresher google
provider.
The old `DataflowJobStatusTrigger` is not a fully correct one, as it was
introduced for Sensor operators, probably, was picked as a “best existing one
that fits here” and new one `DataflowJobStateCompleteTrigger` has a bit
extended logic.
Now, I see your point - some people might not even use def mode and
triggers, so it’s not straightforward for them to update both providers.
The direct solution in my mind is to have a bunch of ugly conditional
imports, if possible I’d prefer to not use it, also if it seems to be a hell to
support, if later triggerer code changes arrive.
I’d appreciate any guidance and tips on how to make these changes properly
and convenient for all provider versions. @eladkal WDYT?
--
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]
