Skip to content

Commit

Permalink
cleanup mock logic
Browse files Browse the repository at this point in the history
  • Loading branch information
hsheth2 committed Dec 24, 2024
1 parent 0af4849 commit dc4845f
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,14 @@ def __init__(
tenant_id: str,
metadata_api_timeout: int,
):
self.__access_token: Optional[str] = None
self.__access_token_expiry_time: Optional[datetime] = None
self.__tenant_id = tenant_id
self._access_token: Optional[str] = None
self._access_token_expiry_time: Optional[datetime] = None

Check warning on line 88 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L87-L88

Added lines #L87 - L88 were not covered by tests

self._tenant_id = tenant_id

Check warning on line 90 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L90

Added line #L90 was not covered by tests
# Test connection by generating access token
logger.info(f"Trying to connect to {self._get_authority_url()}")
# Power-Bi Auth (Service Principal Auth)
self.__msal_client = msal.ConfidentialClientApplication(
self._msal_client = msal.ConfidentialClientApplication(

Check warning on line 94 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L94

Added line #L94 was not covered by tests
client_id,
client_credential=client_secret,
authority=DataResolverBase.AUTHORITY + tenant_id,
Expand Down Expand Up @@ -168,18 +169,18 @@ def _get_app(
pass

def _get_authority_url(self):
return f"{DataResolverBase.AUTHORITY}{self.__tenant_id}"
return f"{DataResolverBase.AUTHORITY}{self._tenant_id}"

Check warning on line 172 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L172

Added line #L172 was not covered by tests

def get_authorization_header(self):
return {Constant.Authorization: self.get_access_token()}

def get_access_token(self):
if self.__access_token is not None and not self._is_access_token_expired():
return self.__access_token
def get_access_token(self) -> str:
if self._access_token is not None and not self._is_access_token_expired():
return self._access_token

Check warning on line 179 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L178-L179

Added lines #L178 - L179 were not covered by tests

logger.info("Generating PowerBi access token")

auth_response = self.__msal_client.acquire_token_for_client(
auth_response = self._msal_client.acquire_token_for_client(

Check warning on line 183 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L183

Added line #L183 was not covered by tests
scopes=[DataResolverBase.SCOPE]
)

Expand All @@ -193,24 +194,24 @@ def get_access_token(self):

logger.info("Generated PowerBi access token")

self.__access_token = "Bearer {}".format(
self._access_token = "Bearer {}".format(

Check warning on line 197 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L197

Added line #L197 was not covered by tests
auth_response.get(Constant.ACCESS_TOKEN)
)
safety_gap = 300
self.__access_token_expiry_time = datetime.now() + timedelta(
self._access_token_expiry_time = datetime.now() + timedelta(

Check warning on line 201 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L201

Added line #L201 was not covered by tests
seconds=(
max(auth_response.get(Constant.ACCESS_TOKEN_EXPIRY, 0) - safety_gap, 0)
)
)

logger.debug(f"{Constant.PBIAccessToken}={self.__access_token}")
logger.debug(f"{Constant.PBIAccessToken}={self._access_token}")

Check warning on line 207 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L207

Added line #L207 was not covered by tests

return self.__access_token
return self._access_token

Check warning on line 209 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L209

Added line #L209 was not covered by tests

def _is_access_token_expired(self) -> bool:
if not self.__access_token_expiry_time:
if not self._access_token_expiry_time:

Check warning on line 212 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L212

Added line #L212 was not covered by tests
return True
return self.__access_token_expiry_time < datetime.now()
return self._access_token_expiry_time < datetime.now()

Check warning on line 214 in metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py

View check run for this annotation

Codecov / codecov/patch

metadata-ingestion/src/datahub/ingestion/source/powerbi/rest_api_wrapper/data_resolver.py#L214

Added line #L214 was not covered by tests

def get_dashboards(self, workspace: Workspace) -> List[Dashboard]:
"""
Expand Down
102 changes: 52 additions & 50 deletions metadata-ingestion/tests/integration/powerbi/test_powerbi.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,18 @@

def mock_msal_cca(*args, **kwargs):
class MsalClient:
call_num = 0
token: Dict[str, Any] = {
"access_token": "dummy",
}
def __init__(self):
self.call_num = 0
self.token: Dict[str, Any] = {
"access_token": "dummy",
}

@staticmethod
def acquire_token_for_client(*args, **kwargs):
MsalClient.call_num += 1
return MsalClient.token
def acquire_token_for_client(self, *args, **kwargs):
self.call_num += 1
return self.token

@staticmethod
def reset():
MsalClient.call_num = 0
def reset(self):
self.call_num = 0

return MsalClient()

Expand Down Expand Up @@ -739,81 +738,84 @@ def test_workspace_container(
)


@mock.patch("msal.ConfidentialClientApplication", side_effect=mock_msal_cca)
def test_access_token_expiry_with_long_expiry(
mock_msal: MagicMock,
pytestconfig: pytest.Config,
tmp_path: str,
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
register_mock_api(pytestconfig=pytestconfig, request_mock=requests_mock)

pipeline = Pipeline.create(
{
"run_id": "powerbi-test",
"source": {
"type": "powerbi",
"config": {
**default_source_config(),
mock_msal = mock_msal_cca()

with mock.patch("msal.ConfidentialClientApplication", return_value=mock_msal):
pipeline = Pipeline.create(
{
"run_id": "powerbi-test",
"source": {
"type": "powerbi",
"config": {
**default_source_config(),
},
},
},
"sink": {
"type": "file",
"config": {
"filename": f"{tmp_path}/powerbi_access_token_mces.json",
"sink": {
"type": "file",
"config": {
"filename": f"{tmp_path}/powerbi_access_token_mces.json",
},
},
},
}
)
}
)

# for long expiry, the token should only be requested once.
MsalClient.token = {
mock_msal.token = {
"access_token": "dummy2",
"expires_in": 3600,
}
mock_msal.reset()

MsalClient.reset()
pipeline.run()
# We expect the token to be requested twice (once for AdminApiResolver and one for RegularApiResolver)
assert MsalClient.call_num == 2
assert mock_msal.call_num == 2


@mock.patch("msal.ConfidentialClientApplication", side_effect=mock_msal_cca)
def test_access_token_expiry_with_short_expiry(
mock_msal: MagicMock,
pytestconfig: pytest.Config,
tmp_path: str,
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
register_mock_api(pytestconfig=pytestconfig, request_mock=requests_mock)

pipeline = Pipeline.create(
{
"run_id": "powerbi-test",
"source": {
"type": "powerbi",
"config": {
**default_source_config(),
mock_msal = mock_msal_cca()
with mock.patch("msal.ConfidentialClientApplication", return_value=mock_msal):
pipeline = Pipeline.create(
{
"run_id": "powerbi-test",
"source": {
"type": "powerbi",
"config": {
**default_source_config(),
},
},
},
"sink": {
"type": "file",
"config": {
"filename": f"{tmp_path}/powerbi_access_token_mces.json",
"sink": {
"type": "file",
"config": {
"filename": f"{tmp_path}/powerbi_access_token_mces.json",
},
},
},
}
)
}
)

# for short expiry, the token should be requested when expires.
MsalClient.token = {
mock_msal.token = {
"access_token": "dummy",
"expires_in": 0,
}
mock_msal.reset()

pipeline.run()
assert MsalClient.call_num > 2
assert mock_msal.call_num > 2


def dataset_type_mapping_set_to_all_platform(pipeline: Pipeline) -> None:
Expand Down

0 comments on commit dc4845f

Please sign in to comment.