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

chore(ingest): remove enable_logging helper #12222

Merged
merged 3 commits into from
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,14 @@
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 @@
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 @@

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
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import logging
import sys
from typing import Any, Dict
from unittest import mock

Expand Down Expand Up @@ -483,12 +481,6 @@ def register_mock_admin_api(request_mock: Any, override_data: dict = {}) -> None
)


def enable_logging():
# set logging to console
logging.getLogger().addHandler(logging.StreamHandler(sys.stdout))
logging.getLogger().setLevel(logging.DEBUG)


def mock_msal_cca(*args, **kwargs):
class MsalClient:
def acquire_token_for_client(self, *args, **kwargs):
Expand Down Expand Up @@ -527,8 +519,6 @@ def default_source_config():
@freeze_time(FROZEN_TIME)
@mock.patch("msal.ConfidentialClientApplication", side_effect=mock_msal_cca)
def test_admin_only_apis(mock_msal, pytestconfig, tmp_path, mock_time, requests_mock):
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_admin_api(request_mock=requests_mock)
Expand Down Expand Up @@ -567,8 +557,6 @@ def test_admin_only_apis(mock_msal, pytestconfig, tmp_path, mock_time, requests_
def test_most_config_and_modified_since(
mock_msal, pytestconfig, tmp_path, mock_time, requests_mock
):
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_admin_api(request_mock=requests_mock)
Expand Down
141 changes: 54 additions & 87 deletions metadata-ingestion/tests/integration/powerbi/test_powerbi.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import datetime
import json
import logging
import re
import sys
from pathlib import Path
from typing import Any, Dict, List, Optional, Union, cast
from unittest import mock
Expand Down Expand Up @@ -31,29 +29,21 @@
FROZEN_TIME = "2022-02-03 07:00:00"


def enable_logging():
# set logging to console
logging.getLogger().addHandler(logging.StreamHandler(sys.stdout))
logging.getLogger().setLevel(logging.DEBUG)


class MsalClient:
call_num = 0
token: Dict[str, Any] = {
"access_token": "dummy",
}

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

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

def reset(self):
self.call_num = 0

def mock_msal_cca(*args, **kwargs):
return MsalClient()


Expand Down Expand Up @@ -154,8 +144,6 @@ def test_powerbi_ingest(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_api(pytestconfig=pytestconfig, request_mock=requests_mock)
Expand Down Expand Up @@ -199,8 +187,6 @@ def test_powerbi_workspace_type_filter(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_api(
Expand Down Expand Up @@ -260,8 +246,6 @@ def test_powerbi_ingest_patch_disabled(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_api(pytestconfig=pytestconfig, request_mock=requests_mock)
Expand Down Expand Up @@ -327,8 +311,6 @@ def test_powerbi_platform_instance_ingest(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_api(pytestconfig=pytestconfig, request_mock=requests_mock)
Expand Down Expand Up @@ -515,8 +497,6 @@ def test_extract_reports(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_api(pytestconfig=pytestconfig, request_mock=requests_mock)
Expand Down Expand Up @@ -561,8 +541,6 @@ def test_extract_lineage(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_api(pytestconfig=pytestconfig, request_mock=requests_mock)
Expand Down Expand Up @@ -660,8 +638,6 @@ def test_admin_access_is_not_allowed(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_api(
Expand Down Expand Up @@ -723,8 +699,6 @@ def test_workspace_container(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"

register_mock_api(pytestconfig=pytestconfig, request_mock=requests_mock)
Expand Down Expand Up @@ -764,85 +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:
enable_logging()

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:
enable_logging()

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 Expand Up @@ -940,8 +913,6 @@ def test_dataset_type_mapping_error(
def test_server_to_platform_map(
mock_msal, pytestconfig, tmp_path, mock_time, requests_mock
):
enable_logging()

test_resources_dir = pytestconfig.rootpath / "tests/integration/powerbi"
new_config: dict = {
**default_source_config(),
Expand Down Expand Up @@ -1416,8 +1387,6 @@ def test_powerbi_cross_workspace_reference_info_message(
mock_time: datetime.datetime,
requests_mock: Any,
) -> None:
enable_logging()

register_mock_api(
pytestconfig=pytestconfig,
request_mock=requests_mock,
Expand Down Expand Up @@ -1495,8 +1464,6 @@ def common_app_ingest(
output_mcp_path: str,
override_config: dict = {},
) -> Pipeline:
enable_logging()

register_mock_api(
pytestconfig=pytestconfig,
request_mock=requests_mock,
Expand Down
Loading
Loading