Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connect_get_namespaced_pod_exec returns _BaseRequestContextManager instead of ClientWebSocketResponse in version 31.x #338

Open
mikelane opened this issue Nov 12, 2024 · 1 comment

Comments

@mikelane
Copy link

Description

When using connect_get_namespaced_pod_exec in version 31.0.1 of kubernetes_asyncio, the function returns an aiohttp.client._BaseRequestContextManager object instead of the expected aiohttp.client_ws.ClientWebSocketResponse. This behavior differs from version 29.0.1, where the correct object (ClientWebSocketResponse) is returned. This change breaks code that relies on the ClientWebSocketResponse, as _BaseRequestContextManager does not implement aiter.

Code Example

The relevant part of the code that triggers the issue:

    async def run_migrations(self, namespace, pod, write):
        ws_client = kube.stream.WsApiClient()
        core_v1_ws = kube.client.CoreV1Api(api_client=ws_client)
        command = ["The command goes here"]
        try:
            exec_res = await core_v1_ws.connect_get_namespaced_pod_exec(
                pod.metadata.name,
                namespace,
                command=command,
                container=RewstApps.graph_api,
                stderr=True,
                stdin=True,
                stdout=True,
                tty=True,
                _preload_content=False,  # return WS, not final content
            )

            try:
                logger.debug('Iterating exec_res')
                logger.debug('type(exec_res): {}', type(exec_res))
                async for line in exec_res:
	                # Rest of code...

Expected Behavior (Version 29.0.1)

The connect_get_namespaced_pod_exec method returns a ClientWebSocketResponse, as shown in the logs:

2024-11-11 20:36:39.929 | DEBUG    | rewst_infra.cli.rewst_k8s_manager:run_migrations:455 - Iterating exec_res
2024-11-11 20:36:39.930 | DEBUG    | rewst_infra.cli.rewst_k8s_manager:run_migrations:456 - type(exec_res): <class 'aiohttp.client_ws.ClientWebSocketResponse'>

Actual Behavior (Version 31.0.1)

The method returns _BaseRequestContextManager instead, causing the code to break since _BaseRequestContextManager does not implement _aiter_:

2024-11-11 20:42:25.184 | DEBUG    | rewst_infra.cli.rewst_k8s_manager:run_migrations:455 - Iterating exec_res
2024-11-11 20:42:25.185 | DEBUG    | rewst_infra.cli.rewst_k8s_manager:run_migrations:456 - type(exec_res): <class 'aiohttp.client._BaseRequestContextManager'>

Impact

The issue causes failures in any logic that depends on the ClientWebSocketResponse object, such as iterating over the WebSocket response. This regression makes it impossible to handle WebSocket streams in the same manner as with version 29.x.

Steps to Reproduce

  1. Use the provided code snippet with kubernetes_asyncio version 31.0.1.
  2. Observe the returned object type from connect_get_namespaced_pod_exec.

Environment

kubernetes_asyncio version: 31.0.1 (issue observed), 29.0.1 (working as expected)
• Python version: 3.12.7
aiohttp version: 3.10.10

Possible Cause

It seems the return value of connect_get_namespaced_pod_exec was unintentionally altered in version 31.x to return a _BaseRequestContextManager instead of a ClientWebSocketResponse. This may be a regression introduced by changes in the kubernetes_asyncio library or its dependencies.

Request

Please investigate and ensure that connect_get_namespaced_pod_exec returns ClientWebSocketResponse as it did in earlier versions. This behavior is essential for handling WebSocket streams correctly.

Workaround

There is currently no clear workaround to resolve this, as the required functionality (_aiter_) is unavailable in _BaseRequestContextManager.

Additional Context

Logs from both versions (29.0.1 and 31.0.1) are included above for reference. If you need further information, I am happy to provide additional details.

@tomplus
Copy link
Owner

tomplus commented Nov 12, 2024

Thank you for detailed description.

I have to confirm that it's a breaking change which was introduced in 31.1.0 [#328]
I should have mentioned about it in the changelog file, I'll do it later today.

To options to adjust your code:

  1. await the context manager to get a response

    exec_res = await exec_res
    logger.debug('Iterating exec_res')
    logger.debug('type(exec_res): {}', type(exec_res))   # << response
  2. use the context manager (recommended)

    async with exec_res as exec_response:
       logger.debug('Iterating exec_response')
       logger.debug('type(exec_response): {}', type(exec_response))   # << response

The second option is recommended as it releases resources properly.

I'm sorry, I hope it'll be easy to apply on your side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants