dstandish commented on code in PR #28546:
URL: https://github.com/apache/airflow/pull/28546#discussion_r1055966660
##########
airflow/kubernetes/pod_generator.py:
##########
@@ -430,6 +419,70 @@ def construct_pod(
except Exception as e:
raise PodReconciliationError from e
+ @classmethod
+ def build_selector_for_k8s_executor_pod(
+ cls,
+ *,
+ dag_id,
+ task_id,
+ try_number,
+ map_index=None,
+ date=None,
+ run_id=None,
+ ):
+ """
+ Generate selector for kubernetes executor pod
+
+ :meta private:
+ """
+ labels = cls.build_labels_for_k8s_executor_pod(
+ dag_id=dag_id,
+ task_id=task_id,
+ try_number=try_number,
+ map_index=map_index,
+ date=date,
+ run_id=run_id,
+ )
+ label_strings = [f"{label_id}={label}" for label_id, label in
sorted(labels.items())]
+ selector = ",".join(label_strings)
+ selector += ",airflow-worker"
+ return selector
+
+ @classmethod
+ def build_labels_for_k8s_executor_pod(
+ cls,
+ *,
+ dag_id,
+ task_id,
+ try_number,
+ airflow_worker=None,
+ map_index=None,
+ date=None,
+ run_id=None,
+ ):
+ """
+ Generate labels for kubernetes executor pod
+
+ :meta private:
+ """
+ labels = {
+ "dag_id": make_safe_label_value(dag_id),
+ "task_id": make_safe_label_value(task_id),
+ "try_number": str(try_number),
+ "kubernetes_executor": "True",
+ }
+ if airflow_worker is not None:
Review Comment:
I don't mind the discussion
I *think* I understand where you're coming from
The way I see it, in this case, the caller must know what it needs and it's
not the labels function to decide. It just builds requested labels in the right
way. If we make the function decide what is required, that's sort of a "logic
in two places" kind of thing. And so, that's why I'm able to use the one labels
func in two ways. It's not worried about the requirements -- that's the callers
job, and different callers have different needs -- one is for pod creation and
the other for pod finding.
Re private, I think it's @uranusjr 's position that meta private means not
part of the public API, not just in terms of doc but also backcompat, and so
that's what I went with here. And it has the pleasant side effect of no
warning about private method accessed when using it in FTH as i do here. And,
really, the fact that it's private is the most important thing for me, because
then the decisions about the interface, you know we can change them whenever so
within reason it doesn't really matter.
--
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]
