c-thiel commented on issue #19135:
URL: https://github.com/apache/airflow/issues/19135#issuecomment-1025495712
Hello @subkanthi,
I just looked through your pull request and have a couple of remarks. Also
from my side thanks for picking this up!!!
1. Regarding the default namespace - should we at least for in-cluster
deployments default to the namespace of the current pod? This is very helpful
if Airflow is run on kubernetes as it will rarely run in the default namespace.
We can determine the namspace from
"/var/run/secrets/kubernetes.io/serviceaccount/namespace"
2. Script Path
Currently the script is always been written to /tmp/script.py. I think it
would be nice if this was a parameter.
3. Pod Name
Should we maybe try to get the dag_id and task_id in there?
I am currently using:
```python
def generate_pod_name(task_id, dag_id=None):
"""Given a task_id and a dag_id, get a kubernetes pod name"""
dag_id = dag_id if dag_id is not None else "xxx-unspecified"
name = f"airflow-task-{_to_k8s_name(dag_id)}-{_to_k8s_name(task_id)}"
name = re.sub(r"[^a-z0-9.-]+", "-", name.lower())
name = name[:62]
if name.endswith("-"):
name = name[:-1]
return name
```
Which is also not perfect I guess. The KubernetesPodOperator itself then
adds another suffix.
4. do_xcom_push
Similar to the VirtualEnvOperator (I think?), we could determine a sensible
default using:
```python
def contains_explicit_return(f):
return any(isinstance(node, ast.Return) for node in
ast.walk(ast.parse(f)))
```
Let me know what you think!
--
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]
