Skip to content

Commit

Permalink
[BUGFIX] FabricPowerBIDatasource 1.0 - TypeError: _PowerBIAsset.build…
Browse files Browse the repository at this point in the history
…_batch_request() got an unexpected keyword argument 'options' (#10318)

Co-authored-by: Gabriel Gore <[email protected]>
  • Loading branch information
tyler-hoffman and Kilo59 authored Aug 30, 2024
1 parent c472ee3 commit 687b5eb
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 5 deletions.
43 changes: 38 additions & 5 deletions great_expectations/experimental/datasource/fabric.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,13 @@
Sorter,
TestConnectionError,
)
from great_expectations.exceptions.exceptions import BuildBatchRequestError

if TYPE_CHECKING:
from great_expectations.core.batch_spec import FabricReaderMethods
from great_expectations.core.partitioners import ColumnPartitioner
from great_expectations.datasource.fluent import BatchParameters
from great_expectations.datasource.fluent.data_connector.batch_filter import BatchSlice
from great_expectations.datasource.fluent.interfaces import (
BatchMetadata,
)
Expand Down Expand Up @@ -114,7 +118,7 @@ def get_batch_list_from_batch_request(self, batch_request: BatchRequest) -> list
)

batch_metadata: BatchMetadata = self._get_batch_metadata_from_batch_request(
batch_request=batch_request
batch_request=batch_request, ignore_options=("dataframe",)
)

batch_list.append(
Expand All @@ -132,13 +136,42 @@ def get_batch_list_from_batch_request(self, batch_request: BatchRequest) -> list
return batch_list

@override
def build_batch_request(self) -> BatchRequest: # type: ignore[override]
def build_batch_request(
self,
options: Optional[BatchParameters] = None,
batch_slice: Optional[BatchSlice] = None,
partitioner: Optional[ColumnPartitioner] = None,
) -> BatchRequest:
"""A batch request that can be used to obtain batches for this DataAsset.
Args:
options: This is not currently supported and must be {} or None for this data asset.
batch_slice: This is not currently supported and must be None for this data asset.
partitioner: This is not currently supported and must be None for this data asset.
Returns:
A BatchRequest object that can be used to obtain a batch list from a Datasource by calling the
get_batch_list_from_batch_request method.
""" # noqa: E501
A `BatchRequest` object that can be used to obtain a batch list from a Datasource by
calling the `get_batch_list_from_batch_request()` method.
"""
asset_type_name: str = self.__class__.__name__
if options:
raise BuildBatchRequestError(
message=f"options is not currently supported for {asset_type_name} "
"and must be None or {}."
)

if batch_slice is not None:
raise BuildBatchRequestError(
message=f"batch_slice is not currently supported for {asset_type_name} "
"and must be None."
)

if partitioner is not None:
raise BuildBatchRequestError(
message=f"partitioner is not currently supported for {asset_type_name} "
"and must be None."
)

return BatchRequest(
datasource_name=self.datasource.name,
data_asset_name=self.name,
Expand Down
60 changes: 60 additions & 0 deletions tests/datasource/fluent/test_fabric.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
from packaging.version import Version

import great_expectations.core.batch_spec
from great_expectations.datasource.fluent import BatchRequest
from great_expectations.exceptions.exceptions import BuildBatchRequestError
from great_expectations.experimental.datasource.fabric import (
FabricPowerBIDatasource,
_PowerBIAsset,
Expand Down Expand Up @@ -137,6 +139,64 @@ def test_reader_options_passthrough(
for asset_kwarg in asset_kwargs:
assert asset_kwarg in captured_kwargs[-1]

@pytest.mark.parametrize(
["asset_type", "asset_kwargs"],
[
param("powerbi_dax", {"dax_string": "my_dax_string"}, id="dax min_args"),
param(
"powerbi_measure",
{
"measure": "my_measure",
},
id="measure min_args",
),
param(
"powerbi_measure",
{
"measure": "my_measure",
"groupby_columns": ["foo[Bar]", "'fizz with space'[Buzz]"],
"filters": {
"State[Region]": ["East", "Central"],
"State[State]": ["WA", "CA"],
},
"fully_qualified_columns": True,
"num_rows": 100,
"use_xmla": True,
},
id="measure all_args",
),
param("powerbi_table", {"table": "my_table_name"}, id="table min_args"),
],
)
def test_valid_build_batch_request_params(
self,
power_bi_datasource: FabricPowerBIDatasource,
asset_type: Literal["powerbi_dax", "powerbi_measure", "powerbi_table"],
asset_kwargs: dict,
):
"""
Test that passing valid keyword parameters to `build_batch_request` does not error.
This test is a bit redundant since mypy should enforce that the method signature is correct,
but we experienced an error here as part of the initial 1.0 release.
"""
add_asset_fn: Callable[[str], _PowerBIAsset] = getattr(
power_bi_datasource, f"add_{asset_type}_asset"
)
my_asset = add_asset_fn(f"my_{asset_type}_asset", **asset_kwargs)

batch_request = my_asset.build_batch_request(
options=None, batch_slice=None, partitioner=None
)
assert isinstance(batch_request, BatchRequest)

def test_invalid_build_batch_request_params(self, power_bi_datasource: FabricPowerBIDatasource):
"""Test that passing invalid parameters to `build_batch_request` raises an error"""
with pytest.raises(BuildBatchRequestError):
power_bi_datasource.add_powerbi_dax_asset(
"my_dax_asset", dax_string="my_dax_string"
).build_batch_request(options={"not": "valid"})


if __name__ == "__main__":
pytest.main([__file__, "-vv"])

0 comments on commit 687b5eb

Please sign in to comment.