shahar1 commented on code in PR #71427:
URL: https://github.com/apache/airflow/pull/71427#discussion_r4090693230
##########
providers/google/docs/operators/cloud/dataproc.rst:
##########
@@ -188,16 +188,12 @@ You can use deferrable mode for this action in order to
run the operator asynchr
Generating Cluster Config
^^^^^^^^^^^^^^^^^^^^^^^^^
-You can also generate **CLUSTER_CONFIG** using functional API,
-this could be easily done using **make()** of
-:class:`~airflow.providers.google.cloud.operators.dataproc.ClusterGenerator`
-You can generate and use config as followed:
-.. exampleinclude::
/../../google/tests/system/google/cloud/dataproc/example_dataproc_cluster_generator.py
- :language: python
- :dedent: 0
- :start-after: [START
how_to_cloud_dataproc_create_cluster_generate_cluster_config]
- :end-before: [END
how_to_cloud_dataproc_create_cluster_generate_cluster_config]
+.. warning::
+ **Deprecated:** The
:class:`~airflow.providers.google.cloud.operators.dataproc.ClusterGenerator`
+ class is deprecated and will be removed after February 10, 2027. Please
pass
Review Comment:
The docs say "after February 10, 2027", but every `planned_removal_date` in
the code says `September 1, 2027`. The code date is the one that satisfies the
6-month minimum in `deprecation-policy.rst`, so please change the docs to match.
##########
providers/google/tests/system/google/cloud/dataproc/example_dataproc_batch_persistent.py:
##########
@@ -53,17 +52,25 @@
CLUSTER_NAME = CLUSTER_NAME_BASE if len(CLUSTER_NAME_FULL) >= 33 else
CLUSTER_NAME_FULL
BATCH_ID = f"batch-{ENV_ID}-{DAG_ID}".replace("_", "-")
-CLUSTER_GENERATOR_CONFIG_FOR_PHS = ClusterGenerator(
- project_id=PROJECT_ID,
- region=REGION,
- master_machine_type="n1-standard-4",
- worker_machine_type="n1-standard-4",
- num_workers=0,
- properties={
- "spark:spark.history.fs.logDirectory": f"gs://{BUCKET_NAME}",
+CLUSTER_CONFIG_FOR_PHS = {
+ "master_config": {
+ "num_instances": 1,
+ "machine_type_uri": "n1-standard-4",
},
- enable_component_gateway=True,
-).make()
+ "worker_config": {
+ "num_instances": 0,
+ "machine_type_uri": "n1-standard-4",
+ },
+ "software_config": {
+ "properties": {
Review Comment:
**Blocking.** The old `ClusterGenerator(..., num_workers=0).make()` also
added `"dataproc:dataproc.allow.zero.workers": "true"` to
`software_config.properties` (it sets `single_node` when `num_workers == 0`,
see `dataproc.py:604-605`). A create request with `worker_config.num_instances:
0` needs this property. Without it, Dataproc treats the cluster as a standard
cluster, which needs at least 2 primary workers, and rejects
`create_cluster_for_phs` ([single-node
clusters](https://cloud.google.com/dataproc/docs/concepts/configuring-clusters/single-node-clusters)).
Apache CI only imports this Dag, so the failure will first show up in the
Google system-test runs.
```suggestion
"software_config": {
"properties": {
"dataproc:dataproc.allow.zero.workers": "true",
```
##########
providers/google/src/airflow/providers/google/cloud/operators/dataproc.py:
##########
@@ -75,6 +76,11 @@
from airflow.providers.common.compat.sdk import Context
+@deprecated(
+ planned_removal_date="September 1, 2027",
+ reason="This helper class is being removed alongside the deprecated
'CreateCluster' class.",
+ category=AirflowProviderDeprecationWarning,
+)
class PreemptibilityType(Enum):
Review Comment:
Users will never see this deprecation. `deprecated` wraps `__new__`, so
member access (`PreemptibilityType.SPOT`) doesn't warn. Only a value lookup
(`PreemptibilityType("SPOT")`) warns, and the one place that does that is
`ClusterGenerator._set_preemptibility_type` (line 387). As a result, every
`ClusterGenerator(...)` call emits a second warning, about a class the user
never touched, and that warning is attributed to `enum.py`. The operator's
keyword-arguments path emits three warnings in total.
I'd drop `@deprecated` from the enum; it is removed together with
`ClusterGenerator` anyway. Optionally, also silence the internal
`ClusterGenerator(**kwargs)` call at line 759, since the operator already emits
its own warning pointing at user code.
##########
providers/google/tests/deprecations_ignore.yml:
##########
@@ -155,3 +155,4 @@
-
providers/google/tests/unit/google/cloud/hooks/test_datacatalog.py::TestCloudDataCatalogMissingProjectIdHook::test_update_tag
-
providers/google/tests/unit/google/cloud/hooks/test_datacatalog.py::TestCloudDataCatalogMissingProjectIdHook::test_update_tag_template
-
providers/google/tests/unit/google/cloud/hooks/test_datacatalog.py::TestCloudDataCatalogMissingProjectIdHook::test_update_tag_template_field
+-
providers/google/tests/unit/google/cloud/operators/test_dataproc.py::TestsClusterGenerator
Review Comment:
This silences the new warnings, but no test checks that they are raised, so
no test fails if the decorators are removed. Recent Google provider
deprecations (#73575, #73354, #56950) each added a
`pytest.warns(AirflowProviderDeprecationWarning, match=...)` test. One
`match="ClusterGenerator"` test would cover this. A
`pytest.mark.filterwarnings` on the class could replace this yml entry.
##########
providers/google/src/airflow/providers/google/cloud/operators/dataproc.py:
##########
@@ -75,6 +76,11 @@
from airflow.providers.common.compat.sdk import Context
+@deprecated(
+ planned_removal_date="September 1, 2027",
+ reason="This helper class is being removed alongside the deprecated
'CreateCluster' class.",
Review Comment:
No class named `CreateCluster` exists. This should probably say
`ClusterGenerator` (same at lines 95 and 119).
##########
providers/google/src/airflow/providers/google/cloud/operators/dataproc.py:
##########
@@ -718,7 +742,7 @@ def __init__(
f"Passing cluster parameters by keywords to
`{type(self).__name__}` will be deprecated. "
"Please provide cluster_config object using `cluster_config`
parameter. "
"You can use
`airflow.dataproc.ClusterGenerator.generate_cluster` "
Review Comment:
This line is edited for the date, but it still tells users to use
`airflow.dataproc.ClusterGenerator.generate_cluster`. That sends them to the
class this PR deprecates, and the path and method don't exist (the method is
`make()`). Please point it at `cluster_config` / `virtual_cluster_config`
instead.
--
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]
