You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/09/29 01:01:09 UTC
[GitHub] [airflow] bdsoha opened a new pull request, #26766: Add sidecar container image to configs
bdsoha opened a new pull request, #26766:
URL: https://github.com/apache/airflow/pull/26766
Adding a configurable value to `PodDefaults.SIDECAR_CONTAINER.image` will allow both:
1. Images hosted on private repositories.
2. Overriding the default `alpine:latest` image with a specific tag.
Closes #10605.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r988508671
##########
tests/providers/cncf/kubernetes/operators/test_kubernetes_pod.py:
##########
@@ -893,6 +894,7 @@ def test_mark_checked_unexpected_exception(self, mock_patch_already_checked, moc
@mock.patch(f"{POD_MANAGER_CLASS}.await_xcom_sidecar_container_start")
def test_wait_for_xcom_sidecar_iff_push_xcom(self, mock_await, mock_extract_xcom, do_xcom_push):
"""Assert we wait for xcom sidecar container if and only if we push xcom."""
+ self.hook_mock.return_value.get_xcom_sidecar_container_image.return_value = "alpine"
Review Comment:
@dstandish without this, the following error is raised:
`yaml.representer.RepresenterError: ('cannot represent an object', <MagicMock name='KubernetesHook().get_xcom_sidecar_container_image().to_dict()' id='139908045486608'>)`
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r985281903
##########
airflow/providers/cncf/kubernetes/utils/xcom_sidecar.py:
##########
@@ -47,13 +47,15 @@ class PodDefaults:
)
-def add_xcom_sidecar(pod: k8s.V1Pod) -> k8s.V1Pod:
+def add_xcom_sidecar(pod: k8s.V1Pod, image: str) -> k8s.V1Pod:
Review Comment:
Moved to `kwargs`
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -368,6 +371,11 @@ def get_namespace(self) -> str | None:
return namespace
return None
+ def get_xcom_sidecar_container_image(self):
+ """Returns the xcom sidecar image that defined in the connection"""
+ image = self._get_field("xcom_sidecar_container_image")
+ return image if image else "alpine"
Review Comment:
No need to append `or None`, as the `_get_field` already returns `None` for missing values.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] raycarter commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by "raycarter (via GitHub)" <gi...@apache.org>.
raycarter commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1647718951
Hello, is this PR in any version released? I can't find the changes in the current version 2.6.3 or in the main-branch.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r988313951
##########
kubernetes_tests/test_kubernetes_pod_operator.py:
##########
@@ -918,6 +918,7 @@ def test_pod_template_file(
):
# todo: This isn't really a system test
await_xcom_sidecar_container_start_mock.return_value = None
+ hook_mock.return_value.get_xcom_sidecar_container_image.return_value = "alpine"
Review Comment:
seems you should not need this change?
##########
tests/providers/cncf/kubernetes/operators/test_kubernetes_pod.py:
##########
@@ -837,6 +837,7 @@ def test_push_xcom_pod_info(
self, mock_await_xcom_sidecar_container_start, mock_extract_xcom, do_xcom_push
):
"""pod name and namespace are *always* pushed; do_xcom_push only controls xcom sidecar"""
+ self.hook_mock.return_value.get_xcom_sidecar_container_image.return_value = "alpine"
Review Comment:
seems you should not need this change?
##########
tests/providers/cncf/kubernetes/operators/test_kubernetes_pod.py:
##########
@@ -893,6 +894,7 @@ def test_mark_checked_unexpected_exception(self, mock_patch_already_checked, moc
@mock.patch(f"{POD_MANAGER_CLASS}.await_xcom_sidecar_container_start")
def test_wait_for_xcom_sidecar_iff_push_xcom(self, mock_await, mock_extract_xcom, do_xcom_push):
"""Assert we wait for xcom sidecar container if and only if we push xcom."""
+ self.hook_mock.return_value.get_xcom_sidecar_container_image.return_value = "alpine"
Review Comment:
seems you should not need this change?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] XD-DENG commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r985117949
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -95,6 +95,9 @@ def get_connection_form_widgets() -> dict[str, Any]:
),
"extra__kubernetes__disable_verify_ssl": BooleanField(lazy_gettext('Disable SSL')),
"extra__kubernetes__disable_tcp_keepalive": BooleanField(lazy_gettext('Disable TCP keepalive')),
+ "extra__kubernetes__sidecar_container_image": StringField(
Review Comment:
I would like to suggest to make the name more explicit, like `extra__kubernetes__xcom_sidecar_container_image`, since it's possible to add other "sidecar" container(s) to the Pods.
Does this make sense to you?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1284186774
@eladkal Thanks for the pointer. Fixed!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] XD-DENG commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1264474430
Triggered to re-run the failed test to see if it's transient
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r984174695
##########
airflow/providers/cncf/kubernetes/utils/xcom_sidecar.py:
##########
@@ -54,6 +56,9 @@ def add_xcom_sidecar(pod: k8s.V1Pod) -> k8s.V1Pod:
pod_cp.spec.volumes.insert(0, PodDefaults.VOLUME)
pod_cp.spec.containers[0].volume_mounts = pod_cp.spec.containers[0].volume_mounts or []
pod_cp.spec.containers[0].volume_mounts.insert(0, PodDefaults.VOLUME_MOUNT)
- pod_cp.spec.containers.append(PodDefaults.SIDECAR_CONTAINER)
+
+ sidecar = copy.deepcopy(PodDefaults.SIDECAR_CONTAINER)
+ sidecar.image = conf.get('kubernetes', 'sidecar_container_image', fallback=sidecar.image)
Review Comment:
stuff in the k8s provider should not refer to core settings. we're trying to keep these separate. core should only be referred to for k8s executor. if you want to be able to modify the xcom container, should do so via airflow connection instead... or... possibly.... through an operator param... but i think airflow connection is better. look in KPO how you can reference attrs on the hook through `self.hook`. this can get you access to params defined in the connection.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish merged pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
dstandish merged PR #26766:
URL: https://github.com/apache/airflow/pull/26766
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r1017457345
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -91,6 +91,9 @@ def get_connection_form_widgets() -> dict[str, Any]:
"cluster_context": StringField(lazy_gettext("Cluster context"), widget=BS3TextFieldWidget()),
"disable_verify_ssl": BooleanField(lazy_gettext("Disable SSL")),
"disable_tcp_keepalive": BooleanField(lazy_gettext("Disable TCP keepalive")),
+ "xcom_sidecar_container_image": StringField(
Review Comment:
@dstandish
Documentation was already included since the initial commit.
Is there anything else you suggest I add?
Have a look [here](https://github.com/bdsoha/airflow/blob/feature/sidecar_container/docs/apache-airflow-providers-cncf-kubernetes/connections/kubernetes.rst#default-connection-ids).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r988508297
##########
kubernetes_tests/test_kubernetes_pod_operator.py:
##########
@@ -918,6 +918,7 @@ def test_pod_template_file(
):
# todo: This isn't really a system test
await_xcom_sidecar_container_start_mock.return_value = None
+ hook_mock.return_value.get_xcom_sidecar_container_image.return_value = "alpine"
Review Comment:
This can be removed.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r985122468
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -95,6 +95,9 @@ def get_connection_form_widgets() -> dict[str, Any]:
),
"extra__kubernetes__disable_verify_ssl": BooleanField(lazy_gettext('Disable SSL')),
"extra__kubernetes__disable_tcp_keepalive": BooleanField(lazy_gettext('Disable TCP keepalive')),
+ "extra__kubernetes__sidecar_container_image": StringField(
Review Comment:
Done
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r1017477697
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -91,6 +91,9 @@ def get_connection_form_widgets() -> dict[str, Any]:
"cluster_context": StringField(lazy_gettext("Cluster context"), widget=BS3TextFieldWidget()),
"disable_verify_ssl": BooleanField(lazy_gettext("Disable SSL")),
"disable_tcp_keepalive": BooleanField(lazy_gettext("Disable TCP keepalive")),
+ "xcom_sidecar_container_image": StringField(
Review Comment:
of course, thanks for your contribution!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r984367835
##########
airflow/providers/cncf/kubernetes/utils/xcom_sidecar.py:
##########
@@ -54,6 +56,9 @@ def add_xcom_sidecar(pod: k8s.V1Pod) -> k8s.V1Pod:
pod_cp.spec.volumes.insert(0, PodDefaults.VOLUME)
pod_cp.spec.containers[0].volume_mounts = pod_cp.spec.containers[0].volume_mounts or []
pod_cp.spec.containers[0].volume_mounts.insert(0, PodDefaults.VOLUME_MOUNT)
- pod_cp.spec.containers.append(PodDefaults.SIDECAR_CONTAINER)
+
+ sidecar = copy.deepcopy(PodDefaults.SIDECAR_CONTAINER)
+ sidecar.image = conf.get('kubernetes', 'sidecar_container_image', fallback=sidecar.image)
Review Comment:
@dstandish I moved the configuration to the connection.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
eladkal commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1283543620
helm tests are failing
https://github.com/apache/airflow/actions/runs/3277418372/jobs/5395247321#step:9:808
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r1017473508
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -91,6 +91,9 @@ def get_connection_form_widgets() -> dict[str, Any]:
"cluster_context": StringField(lazy_gettext("Cluster context"), widget=BS3TextFieldWidget()),
"disable_verify_ssl": BooleanField(lazy_gettext("Disable SSL")),
"disable_tcp_keepalive": BooleanField(lazy_gettext("Disable TCP keepalive")),
+ "xcom_sidecar_container_image": StringField(
Review Comment:
@dstandish Thanks for your guidance with this PR.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
dstandish commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1307724419
ok lemme look
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1307224997
@dstandish @XD-DENG Any we can get this merged? Thanks in advance.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r1017467434
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -91,6 +91,9 @@ def get_connection_form_widgets() -> dict[str, Any]:
"cluster_context": StringField(lazy_gettext("Cluster context"), widget=BS3TextFieldWidget()),
"disable_verify_ssl": BooleanField(lazy_gettext("Disable SSL")),
"disable_tcp_keepalive": BooleanField(lazy_gettext("Disable TCP keepalive")),
+ "xcom_sidecar_container_image": StringField(
Review Comment:
oh, forgive me, looked for it just didn't see it
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1264534115
> Triggered to re-run the failed test to see if it's transient
It passed.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r988533828
##########
tests/providers/cncf/kubernetes/operators/test_kubernetes_pod.py:
##########
@@ -893,6 +914,7 @@ def test_mark_checked_unexpected_exception(self, mock_patch_already_checked, moc
@mock.patch(f"{POD_MANAGER_CLASS}.await_xcom_sidecar_container_start")
def test_wait_for_xcom_sidecar_iff_push_xcom(self, mock_await, mock_extract_xcom, do_xcom_push):
"""Assert we wait for xcom sidecar container if and only if we push xcom."""
+ self.hook_mock.return_value.get_xcom_sidecar_container_image.return_value = "alpine"
Review Comment:
same here, probably should be None right?
```suggestion
self.hook_mock.return_value.get_xcom_sidecar_container_image.return_value = None
```
##########
tests/providers/cncf/kubernetes/operators/test_kubernetes_pod.py:
##########
@@ -837,6 +837,7 @@ def test_push_xcom_pod_info(
self, mock_await_xcom_sidecar_container_start, mock_extract_xcom, do_xcom_push
):
"""pod name and namespace are *always* pushed; do_xcom_push only controls xcom sidecar"""
+ self.hook_mock.return_value.get_xcom_sidecar_container_image.return_value = "alpine"
Review Comment:
Oh i see, yeah, probably better to have it return None then, since here there is no connection being used, so it wouldn't return 'alpine' in this case?
```suggestion
self.hook_mock.return_value.get_xcom_sidecar_container_image.return_value = None
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dstandish commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r1017034008
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -347,6 +350,10 @@ def _get_namespace(self) -> str | None:
return self._get_field("namespace")
return None
+ def get_xcom_sidecar_container_image(self):
+ """Returns the xcom sidecar image that defined in the connection"""
+ return self._get_field("xcom_sidecar_container_image") or None
Review Comment:
```suggestion
return self._get_field("xcom_sidecar_container_image")
```
this is handled by get_field
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -91,6 +91,9 @@ def get_connection_form_widgets() -> dict[str, Any]:
"cluster_context": StringField(lazy_gettext("Cluster context"), widget=BS3TextFieldWidget()),
"disable_verify_ssl": BooleanField(lazy_gettext("Disable SSL")),
"disable_tcp_keepalive": BooleanField(lazy_gettext("Disable TCP keepalive")),
+ "xcom_sidecar_container_image": StringField(
+ lazy_gettext("Xcom sidecar image"), widget=BS3TextFieldWidget()
Review Comment:
```suggestion
lazy_gettext("XCom sidecar image"), widget=BS3TextFieldWidget()
```
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -91,6 +91,9 @@ def get_connection_form_widgets() -> dict[str, Any]:
"cluster_context": StringField(lazy_gettext("Cluster context"), widget=BS3TextFieldWidget()),
"disable_verify_ssl": BooleanField(lazy_gettext("Disable SSL")),
"disable_tcp_keepalive": BooleanField(lazy_gettext("Disable TCP keepalive")),
+ "xcom_sidecar_container_image": StringField(
Review Comment:
can you please add to doc here docs/apache-airflow-providers-cncf-kubernetes/connections/kubernetes.rst
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on pull request #26766: Allow sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1262788848
@jedcunningham The *Tests / Helm: LocalExecutor - v1.24.2* and *Tests / MSSQL2017-latest, Py3.7: API Always CLI Core Integration Other Providers WWW* checks failed due to a timeout.
Is there a way to manually re-run them?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r984323672
##########
airflow/providers/cncf/kubernetes/utils/xcom_sidecar.py:
##########
@@ -54,6 +56,9 @@ def add_xcom_sidecar(pod: k8s.V1Pod) -> k8s.V1Pod:
pod_cp.spec.volumes.insert(0, PodDefaults.VOLUME)
pod_cp.spec.containers[0].volume_mounts = pod_cp.spec.containers[0].volume_mounts or []
pod_cp.spec.containers[0].volume_mounts.insert(0, PodDefaults.VOLUME_MOUNT)
- pod_cp.spec.containers.append(PodDefaults.SIDECAR_CONTAINER)
+
+ sidecar = copy.deepcopy(PodDefaults.SIDECAR_CONTAINER)
+ sidecar.image = conf.get('kubernetes', 'sidecar_container_image', fallback=sidecar.image)
Review Comment:
I will get moving on the changes.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on a diff in pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on code in PR #26766:
URL: https://github.com/apache/airflow/pull/26766#discussion_r988507537
##########
tests/providers/cncf/kubernetes/operators/test_kubernetes_pod.py:
##########
@@ -837,6 +837,7 @@ def test_push_xcom_pod_info(
self, mock_await_xcom_sidecar_container_start, mock_extract_xcom, do_xcom_push
):
"""pod name and namespace are *always* pushed; do_xcom_push only controls xcom sidecar"""
+ self.hook_mock.return_value.get_xcom_sidecar_container_image.return_value = "alpine"
Review Comment:
@dstandish without this, the following error is raised:
`yaml.representer.RepresenterError: ('cannot represent an object', <MagicMock name='KubernetesHook().get_xcom_sidecar_container_image().to_dict()' id='139908045486608'>)`
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1264432589
@XD-DENG @dstandish The *Tests / Postgres10,Py3.7: Always Providers[amazon,cncf.kubernetes,google* check failed for a reason unrelated to the PR.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bdsoha commented on pull request #26766: Allow xcom sidecar container image to be configurable
Posted by GitBox <gi...@apache.org>.
bdsoha commented on PR #26766:
URL: https://github.com/apache/airflow/pull/26766#issuecomment-1296887802
@jedcunningham Any we can get this merged? Thanks in advance.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org