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 2021/01/08 10:46:20 UTC
[GitHub] [airflow] Junnplus opened a new pull request #13563: Fix invalid continue_token for cleanup list pods
Junnplus opened a new pull request #13563:
URL: https://github.com/apache/airflow/pull/13563
```
HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"invalid continue token: continue key is not valid: invalid character '\\u0089' after top-level value","reason":"BadRequest","code":400}
```
Fix invalid continue token when cleanup pods. `_continue` is None will raise above error in kubernetes python 11.0 .
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555775514
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
while True: # pylint: disable=too-many-nested-blocks
pod_list = kube_client.list_namespaced_pod(namespace=namespace, limit=500, _continue=continue_token)
for pod in pod_list.items:
Review comment:
>:param str _continue: The continue option should be set when retrieving more results from the server. Since this value is server defined, clients may only use the continue value from a previous query result with identical query parameters (except for the value of continue) and the server may reject a continue value it does not recognize. If the specified continue value is no longer valid whether due to expiration (generally five to fifteen minutes) or a configuration change on the server, the server will respond with a 410 ResourceExpired error together with a continue token. If the client needs a consistent list, it must restart their list without the continue field. Otherwise, the client may send another list request with the token received with the 410 error, the server will respond with a list starting from the next key, but from the latest snapshot, which is inconsistent from the previous list results - objects that are created, modified, or deleted after the first list
request will be included in the response, as long as their keys are after the \"next key\". This field is not supported when watch is true. Clients may start a watch from the last resourceVersion value returned by the server and not miss any modifications.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555090376
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
@kaxil I remove this k8s test, but I dont know how to do this unit test
continue_token is None in k8s python client 12.x.x is working.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555434904
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
Review comment:
https://github.com/apache/airflow/commit/22415e8271980bd13d88efc9598e20f766ba00d8#diff-b6af878da7897f307bf19ee1137d525e6da262bc2fa6469ce2c40f260be7db48R30
@kaxil continue is None will raise error in this kubernetes test on 11.0, 12.0 is works.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#issuecomment-758292901
[The Workflow run](https://github.com/apache/airflow/actions/runs/477761113) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555769404
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
while True: # pylint: disable=too-many-nested-blocks
pod_list = kube_client.list_namespaced_pod(namespace=namespace, limit=500, _continue=continue_token)
for pod in pod_list.items:
Review comment:
I think this is not pythonic
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555752681
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
while True: # pylint: disable=too-many-nested-blocks
pod_list = kube_client.list_namespaced_pod(namespace=namespace, limit=500, _continue=continue_token)
for pod in pod_list.items:
Review comment:
Wouldn't this be better?
```suggestion
while True: # pylint: disable=too-many-nested-blocks
if continue_token:
pod_list = kube_client.list_namespaced_pod(namespace=namespace, limit=500, _continue=continue_token)
else:
pod_list = kube_client.list_namespaced_pod(namespace=namespace, limit=500)
for pod in pod_list.items:
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555776756
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
while True: # pylint: disable=too-many-nested-blocks
pod_list = kube_client.list_namespaced_pod(namespace=namespace, limit=500, _continue=continue_token)
for pod in pod_list.items:
Review comment:
If you want to go "Pythonic" route, you can do:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555353377
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
Review comment:
@kaxil we don't support python client v12.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555434904
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
Review comment:
https://github.com/apache/airflow/commit/22415e8271980bd13d88efc9598e20f766ba00d8#diff-b6af878da7897f307bf19ee1137d525e6da262bc2fa6469ce2c40f260be7db48R30
@kaxil continue is None will raise error in this kubernetes test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555432932
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
@mik-laj I know we use kubernetes python client is 11.0
> _continue is None will raise above error in kubernetes python 11.0 .
@kaxil I patch this on my production, it is working. so I write a kubernetes test fot it.
for 11.0
https://raw.githubusercontent.com/kubernetes-client/python/v11.0.0/kubernetes/client/api/core_v1_api.py
```
def list_pod_for_all_namespaces_with_http_info(self, **kwargs): # noqa: E501
...
if '_continue' in local_var_params:
query_params.append(('continue', local_var_params['_continue'])) # noqa: E501
```
for 12.0
https://raw.githubusercontent.com/kubernetes-client/python/v12.0.0/kubernetes/client/api/core_v1_api.py
```
def list_pod_for_all_namespaces_with_http_info(self, **kwargs): # noqa: E501
```
if '_continue' in local_var_params and local_var_params['_continue'] is not None: # noqa: E501
query_params.append(('continue', local_var_params['_continue'])) # noqa: E501
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #13563:
URL: https://github.com/apache/airflow/pull/13563
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r554236229
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
We already have tests at https://github.com/apache/airflow/blob/9d6c7487dd03a42d1b6a1f19fae0c9a746139dbe/tests/cli/commands/test_kubernetes_command.py#L57-L174
Can you add a test over there please instead of this new file
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r554236229
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
We already have tests at https://github.com/apache/airflow/blob/9d6c7487dd03a42d1b6a1f19fae0c9a746139dbe/tests/cli/commands/test_kubernetes_command.py#L57-L174
Can you add a test over there please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555778217
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
while True: # pylint: disable=too-many-nested-blocks
pod_list = kube_client.list_namespaced_pod(namespace=namespace, limit=500, _continue=continue_token)
for pod in pod_list.items:
Review comment:
Btw I have no problem with the current suggestion too of using an empty string (`''`) , but want to make sure it works with 11.0 and 12.0 (we will support 12.0 soon -- or at least in near future)
Personally, I would not set it for the first iteration but it does not make a difference
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555353217
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
We don't support python client 12.0.0. https://github.com/apache/airflow/blob/065c02b3614d7839c0bf62c5aa467cac59293aea/constraints-3.8.txt#L223 See also: https://github.com/kubernetes-client/python/issues/1304
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555131890
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
Review comment:
Are you sure an empty string works ?
https://github.com/kubernetes-client/python/blob/v11.0.0/kubernetes/client/models/v1_list_meta.py#L56-L57
https://github.com/kubernetes-client/python/blob/v12.0.0/kubernetes/client/models/v1_list_meta.py#L61-L62
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r554236229
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
We already have tests at https://github.com/apache/airflow/blob/9d6c7487dd03a42d1b6a1f19fae0c9a746139dbe/tests/cli/commands/test_kubernetes_command.py#L57-L174
Can you add a test over there please
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
We already have tests at https://github.com/apache/airflow/blob/9d6c7487dd03a42d1b6a1f19fae0c9a746139dbe/tests/cli/commands/test_kubernetes_command.py#L57-L174
Can you add a test over there please instead of this new file
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r554993054
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
@Junnplus You are also mocking `get_kube_client` in the test, so it is not run on the K8s cluster
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555820009
##########
File path: airflow/cli/commands/kubernetes_command.py
##########
@@ -90,7 +90,7 @@ def cleanup_pods(args):
print('Loading Kubernetes configuration')
kube_client = get_kube_client()
print(f'Listing pods in namespace {namespace}')
- continue_token = None
+ continue_token = ''
while True: # pylint: disable=too-many-nested-blocks
pod_list = kube_client.list_namespaced_pod(namespace=namespace, limit=500, _continue=continue_token)
for pod in pod_list.items:
Review comment:
@kaxil Thank you for your suggestion, I like this code, it is easier to test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r554287153
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
I think this test different with unit tests, this case maybe test with Kubernetes
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555432932
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
@mik-laj I know we use kubernetes python client is 11.0
> _continue is None will raise above error in kubernetes python 11.0 .
I patch this on my production, it is working. so I write a kubernetes test fot it.
for 11.0
https://raw.githubusercontent.com/kubernetes-client/python/v11.0.0/kubernetes/client/api/core_v1_api.py
```
def list_pod_for_all_namespaces_with_http_info(self, **kwargs): # noqa: E501
...
if '_continue' in local_var_params:
query_params.append(('continue', local_var_params['_continue'])) # noqa: E501
```
for 12.0
https://raw.githubusercontent.com/kubernetes-client/python/v12.0.0/kubernetes/client/api/core_v1_api.py
```
def list_pod_for_all_namespaces_with_http_info(self, **kwargs): # noqa: E501
```
if '_continue' in local_var_params and local_var_params['_continue'] is not None: # noqa: E501
query_params.append(('continue', local_var_params['_continue'])) # noqa: E501
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555432932
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
@mik-laj I know we use kubernetes python client is 11.0
> _continue is None will raise above error in kubernetes python 11.0 .
@kaxil I patch this on my production, it is working. so I write a kubernetes test fot it.
for 11.0
https://raw.githubusercontent.com/kubernetes-client/python/v11.0.0/kubernetes/client/api/core_v1_api.py
```
def list_pod_for_all_namespaces_with_http_info(self, **kwargs): # noqa: E501
...
if '_continue' in local_var_params:
query_params.append(('continue', local_var_params['_continue'])) # noqa: E501
```
for 12.0
https://raw.githubusercontent.com/kubernetes-client/python/v12.0.0/kubernetes/client/api/core_v1_api.py
```
def list_pod_for_all_namespaces_with_http_info(self, **kwargs): # noqa: E501
...
if '_continue' in local_var_params and local_var_params['_continue'] is not None: # noqa: E501
query_params.append(('continue', local_var_params['_continue'])) # noqa: E501
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r554287153
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
I think this test different with unit tests, this case maybe test with Kubernetes
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555408739
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
https://github.com/apache/airflow/pull/13563#discussion_r555131890
https://github.com/kubernetes-client/python/blob/v11.0.0/kubernetes/client/models/v1_list_meta.py#L56-L57
https://github.com/kubernetes-client/python/blob/v12.0.0/kubernetes/client/models/v1_list_meta.py#L61-L62
The code looks same for v11.0.0 and v12.0.0 anyways.. so are you sure an empty string works @Junnplus ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555432932
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
@mik-laj I know we use kubernetes python client is 11.0
> _continue is None will raise above error in kubernetes python 11.0 .
for 11.0
https://raw.githubusercontent.com/kubernetes-client/python/v11.0.0/kubernetes/client/api/core_v1_api.py
```
def list_pod_for_all_namespaces_with_http_info(self, **kwargs): # noqa: E501
...
if '_continue' in local_var_params:
query_params.append(('continue', local_var_params['_continue'])) # noqa: E501
```
for 12.0
https://raw.githubusercontent.com/kubernetes-client/python/v12.0.0/kubernetes/client/api/core_v1_api.py
```
def list_pod_for_all_namespaces_with_http_info(self, **kwargs): # noqa: E501
```
if '_continue' in local_var_params and local_var_params['_continue'] is not None: # noqa: E501
query_params.append(('continue', local_var_params['_continue'])) # noqa: E501
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555090376
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
@kaxil I remove this k8s test, but I dont know how to do this unit test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Junnplus commented on a change in pull request #13563: Fix invalid continue_token for cleanup list pods
Posted by GitBox <gi...@apache.org>.
Junnplus commented on a change in pull request #13563:
URL: https://github.com/apache/airflow/pull/13563#discussion_r555090376
##########
File path: kubernetes_tests/test_cleanup_pods.py
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from unittest import mock
+
+from airflow.cli import cli_parser
+from airflow.cli.commands import kubernetes_command
+from airflow.kubernetes.kube_client import get_kube_client
+
+
+class TestCleanupPods(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ cls.parser = cli_parser.get_parser()
+
+ @mock.patch("airflow.cli.commands.kubernetes_command.get_kube_client")
+ def test_cleanup_pods_working(self, mock_client):
+ mock_client.return_value = get_kube_client(in_cluster=False)
+ kubernetes_command.cleanup_pods(
+ self.parser.parse_args(['kubernetes', 'cleanup-pods', '--namespace', 'test-namespace'])
+ )
Review comment:
@kaxil I remove this k8s test, but I dont know how to do this unit test
continue_token is None in k8s python client 12 is working.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org