Skip to content

Commit 088ce83

Browse files
committed
fix copilot comments
1 parent e808b8a commit 088ce83

File tree

3 files changed

+34
-13
lines changed

3 files changed

+34
-13
lines changed

libs/foundry-dev-tools/src/foundry_dev_tools/clients/foundry_sql_server.py

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -368,24 +368,24 @@ def query_foundry_sql(
368368
def query_foundry_sql(
369369
self,
370370
query: str,
371-
return_type: SQLReturnType = ...,
371+
return_type: Literal["pandas", "polars", "spark", "arrow"] = ...,
372372
branch: Ref = ...,
373373
sql_dialect: FurnaceSqlDialect = ...,
374374
arrow_compression_codec: ArrowCompressionCodec = ...,
375375
timeout: int = ...,
376376
experimental_use_trino: bool = ...,
377-
) -> tuple[dict, list[list]] | pd.core.frame.DataFrame | pl.DataFrame | pa.Table | pyspark.sql.DataFrame: ...
377+
) -> pd.core.frame.DataFrame | pl.DataFrame | pa.Table | pyspark.sql.DataFrame: ...
378378

379379
def query_foundry_sql(
380380
self,
381381
query: str,
382-
return_type: SQLReturnType = "pandas",
382+
return_type: Literal["pandas", "polars", "spark", "arrow"] = "pandas",
383383
branch: Ref = "master",
384384
sql_dialect: FurnaceSqlDialect = "SPARK",
385385
arrow_compression_codec: ArrowCompressionCodec = "NONE",
386386
timeout: int = 600,
387387
experimental_use_trino: bool = False,
388-
) -> tuple[dict, list[list]] | pd.core.frame.DataFrame | pl.DataFrame | pa.Table | pyspark.sql.DataFrame:
388+
) -> pd.core.frame.DataFrame | pl.DataFrame | pa.Table | pyspark.sql.DataFrame:
389389
"""Queries the Foundry SQL server using the V2 API.
390390
391391
Uses Arrow IPC to communicate with the Foundry SQL Server Endpoint.
@@ -397,7 +397,7 @@ def query_foundry_sql(
397397
398398
Args:
399399
query: The SQL Query
400-
return_type: See :py:class:foundry_dev_tools.foundry_api_client.SQLReturnType
400+
return_type: The return type (pandas, polars, spark, or arrow). Note: "raw" is not supported in V2.
401401
branch: The dataset branch to query
402402
sql_dialect: The SQL dialect to use (only SPARK is supported for V2)
403403
arrow_compression_codec: Arrow compression codec (NONE, LZ4, ZSTD)
@@ -414,13 +414,15 @@ def query_foundry_sql(
414414
FoundrySqlQueryClientTimedOutError: If the query times out
415415
416416
""" # noqa: E501
417-
assert_in_literal(sql_dialect, FurnaceSqlDialect, "sql_dialect")
418-
419417
if experimental_use_trino:
420418
query = query.replace("SELECT ", "SELECT /*+ backend(trino) */ ", 1)
421419

422420
response_json = self.api_query(
423-
query=query, dialect=sql_dialect, branch=branch, arrow_compression_codec=arrow_compression_codec
421+
query=query,
422+
dialect=sql_dialect,
423+
branch=branch,
424+
arrow_compression_codec=arrow_compression_codec,
425+
timeout=timeout,
424426
).json()
425427

426428
query_handle = self._extract_query_handle(response_json)
@@ -466,7 +468,11 @@ def query_foundry_sql(
466468
if return_type == "arrow":
467469
return arrow_stream_reader.read_all()
468470

469-
raise ValueError("The following return_type is not supported: " + return_type)
471+
msg = (
472+
f"Unsupported return_type: {return_type}. "
473+
f"V2 API supports: pandas, polars, spark, arrow (raw is not supported)"
474+
)
475+
raise ValueError(msg)
470476

471477
def _extract_query_handle(self, response_json: dict[str, Any]) -> dict[str, Any]:
472478
"""Extract query handle from execute response.
@@ -526,6 +532,7 @@ def api_query(
526532
dialect: FurnaceSqlDialect,
527533
branch: Ref,
528534
arrow_compression_codec: ArrowCompressionCodec = "NONE",
535+
timeout: int = 600,
529536
**kwargs,
530537
) -> requests.Response:
531538
"""Execute a SQL query via the V2 API.
@@ -535,12 +542,16 @@ def api_query(
535542
dialect: The SQL dialect to use (only SPARK is supported)
536543
branch: The dataset branch to query
537544
arrow_compression_codec: Arrow compression codec (NONE, LZ4, ZSTD)
545+
timeout: Query timeout in seconds (used for error context)
538546
**kwargs: gets passed to :py:meth:`APIClient.api_request`
539547
540548
Returns:
541549
Response with query handle and initial status
542550
543551
"""
552+
assert_in_literal(dialect, FurnaceSqlDialect, "dialect")
553+
assert_in_literal(arrow_compression_codec, ArrowCompressionCodec, "arrow_compression_codec")
554+
544555
return self.api_request(
545556
"POST",
546557
"sql-endpoint/v1/queries/query",
@@ -557,6 +568,7 @@ def api_query(
557568
"resultMode": "AUTO",
558569
},
559570
},
571+
error_handling=ErrorHandlingConfig(branch=branch, dialect=dialect, timeout=timeout),
560572
**kwargs,
561573
)
562574

tests/integration/clients/test_foundry_sql_server.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ def test_v2_return_type_arrow():
134134

135135
def test_v2_return_type_raw_not_supported():
136136
"""Test V2 client with raw return type."""
137-
with pytest.raises(ValueError, match="The following return_type is not supported: .+"):
138-
schema, rows = TEST_SINGLETON.ctx.foundry_sql_server_v2.query_foundry_sql(
137+
with pytest.raises(ValueError, match="Unsupported return_type: raw"):
138+
TEST_SINGLETON.ctx.foundry_sql_server_v2.query_foundry_sql(
139139
query=f"SELECT sepal_width, sepal_length FROM `{TEST_SINGLETON.iris_new.rid}` LIMIT 3",
140-
return_type="raw",
140+
return_type="raw", # type: ignore[arg-type]
141141
)
142142

143143

tests/unit/clients/test_foundry_sql_server.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,17 @@ def test_v2_poll_for_query_completion_timeout(mocker, test_context_mock):
219219

220220
def test_v2_ansi_dialect_not_supported(test_context_mock):
221221
"""Test that V2 client rejects ANSI SQL dialect."""
222-
with pytest.raises(TypeError, match="'ANSI' is not a valid option for sql_dialect"):
222+
with pytest.raises(TypeError, match="'ANSI' is not a valid option for dialect"):
223223
test_context_mock.foundry_sql_server_v2.query_foundry_sql(
224224
"SELECT * FROM `ri.foundry.main.dataset.test-dataset`",
225225
sql_dialect="ANSI", # type: ignore[arg-type]
226226
)
227+
228+
229+
def test_v2_invalid_compression_codec(test_context_mock):
230+
"""Test that V2 client rejects invalid arrow compression codec."""
231+
with pytest.raises(TypeError, match="'INVALID' is not a valid option for arrow_compression_codec"):
232+
test_context_mock.foundry_sql_server_v2.query_foundry_sql(
233+
"SELECT * FROM `ri.foundry.main.dataset.test-dataset`",
234+
arrow_compression_codec="INVALID", # type: ignore[arg-type]
235+
)

0 commit comments

Comments
 (0)