Skip to content

Commit a05dc82

Browse files
vuanhphungIsaac
andcommitted
test(thrift): assert the buffered reply is released after make_request
Locks in that the transport's reply buffer is cleared once a successful RPC returns (make_request closes the transport in its finally block). Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 124cc5d commit a05dc82

1 file changed

Lines changed: 50 additions & 0 deletions

File tree

‎tests/unit/test_thrift_backend.py‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,56 @@ def test_http_client_cstringio_refill_on_truncated_reply_raises(self):
443443
with self.assertRaises(EOFError):
444444
http_client.cstringio_refill(b"", 4)
445445

446+
def test_reply_buffer_is_released_after_successful_make_request(self):
447+
"""make_request closes the transport on success too, so the buffered
448+
reply body is not kept on the connection after it has been decoded."""
449+
from thrift.protocol import TBinaryProtocol
450+
from thrift.transport import TTransport
451+
from databricks.sql.thrift_api.TCLIService import TCLIService
452+
453+
batch = b"x" * (1024 * 1024)
454+
resp = ttypes.TFetchResultsResp(
455+
status=ttypes.TStatus(statusCode=ttypes.TStatusCode.SUCCESS_STATUS),
456+
hasMoreRows=False,
457+
results=ttypes.TRowSet(
458+
startRowOffset=0,
459+
rows=[],
460+
arrowBatches=[ttypes.TSparkArrowBatch(batch=batch, rowCount=1)],
461+
),
462+
)
463+
reply = TTransport.TMemoryBuffer()
464+
reply_proto = TBinaryProtocol.TBinaryProtocol(reply)
465+
reply_proto.writeMessageBegin("FetchResults", 2, 0) # REPLY
466+
TCLIService.FetchResults_result(success=resp).write(reply_proto)
467+
reply_proto.writeMessageEnd()
468+
469+
thrift_backend = ThriftDatabricksClient(
470+
"foobar",
471+
443,
472+
"path",
473+
[("User-Agent", "test")],
474+
auth_provider=AuthProvider(),
475+
ssl_options=SSLOptions(),
476+
http_client=MagicMock(),
477+
)
478+
mock_resp = Mock(status=200, reason="OK", headers={})
479+
mock_resp.read.return_value = reply.getvalue()
480+
thrift_backend._transport._THttpClient__pool = Mock()
481+
thrift_backend._transport._THttpClient__pool.request.return_value = mock_resp
482+
483+
op_handle = ttypes.TOperationHandle(
484+
operationId=ttypes.THandleIdentifier(guid=b"g" * 16, secret=b"s" * 16),
485+
operationType=ttypes.TOperationType.EXECUTE_STATEMENT,
486+
hasResultSet=True,
487+
)
488+
result = thrift_backend.make_request(
489+
thrift_backend._client.FetchResults,
490+
ttypes.TFetchResultsReq(operationHandle=op_handle, orientation=0, maxRows=1),
491+
)
492+
493+
self.assertEqual(result.results.arrowBatches[0].batch, batch)
494+
self.assertEqual(thrift_backend._transport.cstringio_buf.getvalue(), b"")
495+
446496
@patch("databricks.sql.backend.thrift_backend.unsafe_logger")
447497
@patch("databricks.sql.auth.thrift_http_client.THttpClient")
448498
def test_unsafe_logger_formats_payloads_lazily(

0 commit comments

Comments
 (0)