jedcunningham commented on code in PR #32767:
URL: https://github.com/apache/airflow/pull/32767#discussion_r1272734231


##########
airflow/kubernetes/pre_2_7_compatibility/__init__.py:
##########
@@ -15,24 +14,17 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-"""
-This module is deprecated.
-Please use :mod:`kubernetes.client.models` for `V1ResourceRequirements` and 
`Port`.
-"""
 from __future__ import annotations
 
+# All the classes in this module should only be kept for 
backwards-compatibility reasons.
+# old cncf.kubernetes providers will use those in their frozen version for 
pre-2.7.0 release
 import warnings
 
-from airflow.exceptions import RemovedInAirflow3Warning
-
-# flake8: noqa
-
-with warnings.catch_warnings():
-    warnings.simplefilter("ignore", RemovedInAirflow3Warning)
-    from airflow.providers.cncf.kubernetes.backcompat.pod import Port, 
Resources
-
 warnings.warn(
-    "This module is deprecated. Please use `kubernetes.client.models` for 
`V1ResourceRequirements` and `Port`.",
-    RemovedInAirflow3Warning,
+    "This module is deprecated. The `cncf.kubernetes` provider before version 
2.7.0 use this module or you"

Review Comment:
   Is this meant to be "before version 7.4.0"?



##########
airflow/kubernetes/pre_2_7_compatibility/pod_generator_deprecated.py:
##########
@@ -64,7 +64,7 @@ def make_safe_label_value(string):
 
     Valid label values must be 63 characters or less and must be empty or 
begin and
     end with an alphanumeric character ([a-z0-9A-Z]) with dashes (-), 
underscores (_),
-    dots (.), and alphanumerics between.
+    dots (.), and alpha-numbers between.

Review Comment:
   This change sounds weird to me, why move away from alphanumerics? 



##########
airflow/models/taskinstance.py:
##########
@@ -2288,32 +2275,51 @@ def render_templates(self, context: Context | None = 
None) -> Operator:
         return original_task
 
     def render_k8s_pod_yaml(self) -> dict | None:
-        """Render k8s pod yaml."""
-        from kubernetes.client.api_client import ApiClient
-
-        from airflow.kubernetes.kube_config import KubeConfig
-        from airflow.kubernetes.kubernetes_helper_functions import 
create_pod_id  # Circular import
-        from airflow.kubernetes.pod_generator import PodGenerator
+        """Render the k8s pod yaml."""
+        try:
+            from airflow.providers.cncf.kubernetes.template_rendering import (
+                render_k8s_pod_yaml as render_k8s_pod_yaml_from_provider,
+            )
+        except ImportError:
+            raise RuntimeError(
+                "You need to have 'cncf.kubernetes' provider installed to use 
this feature. "
+                "Also rather than calling it directly you should import "
+                "render_k8s_pod_yaml from 
airflow.providers.cncf.kubernetes.template_rendering "
+                "and call id with TaskInstance as the first argument."
+            )
+        warnings.warn(
+            "You should not call task_instance.render_k8s_pod_yaml directly. 
This method will be removed"
+            "in Airflow 3. Rather than calling it directly you should import "
+            "render_k8s_pod_yaml from 
airflow.providers.cncf.kubernetes.template_rendering "
+            "and call id with TaskInstance as the first argument.",

Review Comment:
   ```suggestion
               "and call it with TaskInstance as the first argument.",
   ```



##########
docs/spelling_wordlist.txt:
##########
@@ -296,6 +296,7 @@ ContainerPort
 contentUrl
 contextmgr
 contrib
+CoreV

Review Comment:
   This is a little suspicious...



##########
airflow/models/taskinstance.py:
##########
@@ -2288,32 +2275,51 @@ def render_templates(self, context: Context | None = 
None) -> Operator:
         return original_task
 
     def render_k8s_pod_yaml(self) -> dict | None:
-        """Render k8s pod yaml."""
-        from kubernetes.client.api_client import ApiClient
-
-        from airflow.kubernetes.kube_config import KubeConfig
-        from airflow.kubernetes.kubernetes_helper_functions import 
create_pod_id  # Circular import
-        from airflow.kubernetes.pod_generator import PodGenerator
+        """Render the k8s pod yaml."""
+        try:
+            from airflow.providers.cncf.kubernetes.template_rendering import (
+                render_k8s_pod_yaml as render_k8s_pod_yaml_from_provider,
+            )
+        except ImportError:
+            raise RuntimeError(
+                "You need to have 'cncf.kubernetes' provider installed to use 
this feature. "
+                "Also rather than calling it directly you should import "
+                "render_k8s_pod_yaml from 
airflow.providers.cncf.kubernetes.template_rendering "
+                "and call id with TaskInstance as the first argument."
+            )
+        warnings.warn(
+            "You should not call task_instance.render_k8s_pod_yaml directly. 
This method will be removed"
+            "in Airflow 3. Rather than calling it directly you should import "
+            "render_k8s_pod_yaml from 
airflow.providers.cncf.kubernetes.template_rendering "
+            "and call id with TaskInstance as the first argument.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return render_k8s_pod_yaml_from_provider(self)
 
-        kube_config = KubeConfig()
-        pod = PodGenerator.construct_pod(
-            dag_id=self.dag_id,
-            run_id=self.run_id,
-            task_id=self.task_id,
-            map_index=self.map_index,
-            date=None,
-            pod_id=create_pod_id(self.dag_id, self.task_id),
-            try_number=self.try_number,
-            kube_image=kube_config.kube_image,
-            args=self.command_as_list(),
-            pod_override_object=PodGenerator.from_obj(self.executor_config),
-            scheduler_job_id="0",
-            namespace=kube_config.executor_namespace,
-            
base_worker_pod=PodGenerator.deserialize_model_file(kube_config.pod_template_file),
-            with_mutation_hook=True,
+    @provide_session
+    def get_rendered_k8s_spec(self, session: Session = NEW_SESSION):
+        """Render the k8s pod yaml."""
+        try:
+            from airflow.providers.cncf.kubernetes.template_rendering import (
+                get_rendered_k8s_spec as get_rendered_k8s_spec_from_provider,
+            )
+        except ImportError:
+            raise RuntimeError(
+                "You need to have 'cncf.kubernetes' provider installed to use 
this feature. "
+                "Also rather than calling it directly you should import "
+                "get_rendered_k8s_spec from 
airflow.providers.cncf.kubernetes.template_rendering "
+                "and call id with TaskInstance as the first argument."
+            )
+        warnings.warn(
+            "You should not call task_instance.render_k8s_pod_yaml directly. 
This method will be removed"
+            "in Airflow 3. Rather than calling it directly you should import "
+            "get_rendered_k8s_spec from 
airflow.providers.cncf.kubernetes.template_rendering "
+            "and call id with TaskInstance as the first argument.",

Review Comment:
   ```suggestion
               "and call it with TaskInstance as the first argument.",
   ```



##########
airflow/models/taskinstance.py:
##########
@@ -2288,32 +2275,51 @@ def render_templates(self, context: Context | None = 
None) -> Operator:
         return original_task
 
     def render_k8s_pod_yaml(self) -> dict | None:
-        """Render k8s pod yaml."""
-        from kubernetes.client.api_client import ApiClient
-
-        from airflow.kubernetes.kube_config import KubeConfig
-        from airflow.kubernetes.kubernetes_helper_functions import 
create_pod_id  # Circular import
-        from airflow.kubernetes.pod_generator import PodGenerator
+        """Render the k8s pod yaml."""
+        try:
+            from airflow.providers.cncf.kubernetes.template_rendering import (
+                render_k8s_pod_yaml as render_k8s_pod_yaml_from_provider,
+            )
+        except ImportError:
+            raise RuntimeError(
+                "You need to have 'cncf.kubernetes' provider installed to use 
this feature. "
+                "Also rather than calling it directly you should import "
+                "render_k8s_pod_yaml from 
airflow.providers.cncf.kubernetes.template_rendering "
+                "and call id with TaskInstance as the first argument."

Review Comment:
   ```suggestion
                   "and call it with TaskInstance as the first argument."
   ```



##########
airflow/exceptions.py:
##########
@@ -375,12 +375,28 @@ class TaskDeferralError(AirflowException):
     """Raised when a task failed during deferral for some reason."""
 
 
-class PodMutationHookException(AirflowException):
-    """Raised when exception happens during Pod Mutation Hook execution."""
-
-
-class PodReconciliationError(AirflowException):
-    """Raised when an error is encountered while trying to merge pod 
configs."""
+# The try/except handling is needed after we moved all k8s classes to 
cncf.kubernetes provider
+# These two exceptions are used internally by Kubernetes Executor put also by 
PodGenerator, so we need

Review Comment:
   ```suggestion
   # These two exceptions are used internally by Kubernetes Executor but also 
by PodGenerator, so we need
   ```



##########
airflow/models/taskinstance.py:
##########
@@ -2288,32 +2275,51 @@ def render_templates(self, context: Context | None = 
None) -> Operator:
         return original_task
 
     def render_k8s_pod_yaml(self) -> dict | None:
-        """Render k8s pod yaml."""
-        from kubernetes.client.api_client import ApiClient
-
-        from airflow.kubernetes.kube_config import KubeConfig
-        from airflow.kubernetes.kubernetes_helper_functions import 
create_pod_id  # Circular import
-        from airflow.kubernetes.pod_generator import PodGenerator
+        """Render the k8s pod yaml."""
+        try:
+            from airflow.providers.cncf.kubernetes.template_rendering import (
+                render_k8s_pod_yaml as render_k8s_pod_yaml_from_provider,
+            )
+        except ImportError:
+            raise RuntimeError(
+                "You need to have 'cncf.kubernetes' provider installed to use 
this feature. "
+                "Also rather than calling it directly you should import "
+                "render_k8s_pod_yaml from 
airflow.providers.cncf.kubernetes.template_rendering "
+                "and call id with TaskInstance as the first argument."
+            )
+        warnings.warn(
+            "You should not call task_instance.render_k8s_pod_yaml directly. 
This method will be removed"
+            "in Airflow 3. Rather than calling it directly you should import "
+            "render_k8s_pod_yaml from 
airflow.providers.cncf.kubernetes.template_rendering "
+            "and call id with TaskInstance as the first argument.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return render_k8s_pod_yaml_from_provider(self)
 
-        kube_config = KubeConfig()
-        pod = PodGenerator.construct_pod(
-            dag_id=self.dag_id,
-            run_id=self.run_id,
-            task_id=self.task_id,
-            map_index=self.map_index,
-            date=None,
-            pod_id=create_pod_id(self.dag_id, self.task_id),
-            try_number=self.try_number,
-            kube_image=kube_config.kube_image,
-            args=self.command_as_list(),
-            pod_override_object=PodGenerator.from_obj(self.executor_config),
-            scheduler_job_id="0",
-            namespace=kube_config.executor_namespace,
-            
base_worker_pod=PodGenerator.deserialize_model_file(kube_config.pod_template_file),
-            with_mutation_hook=True,
+    @provide_session
+    def get_rendered_k8s_spec(self, session: Session = NEW_SESSION):
+        """Render the k8s pod yaml."""
+        try:
+            from airflow.providers.cncf.kubernetes.template_rendering import (
+                get_rendered_k8s_spec as get_rendered_k8s_spec_from_provider,
+            )
+        except ImportError:
+            raise RuntimeError(
+                "You need to have 'cncf.kubernetes' provider installed to use 
this feature. "
+                "Also rather than calling it directly you should import "
+                "get_rendered_k8s_spec from 
airflow.providers.cncf.kubernetes.template_rendering "
+                "and call id with TaskInstance as the first argument."

Review Comment:
   ```suggestion
                   "and call it with TaskInstance as the first argument."
   ```



##########
airflow/providers/celery/CHANGELOG.rst:
##########
@@ -33,7 +33,22 @@ Changelog
 .. note::
   This provider release is the first release that has Celery Executor and
   Celery Kubernetes Executor moved from the core ``apache-airflow`` package to 
a Celery
-  provider package.
+  provider package. expects ``apache-airflow-providers-cncf-kubernetes`` in 
version 7.4.0+ installed

Review Comment:
   ```suggestion
     provider package. It also expects 
``apache-airflow-providers-cncf-kubernetes`` in version 7.4.0+ installed
   ```



-- 
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]

Reply via email to