-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat(ingest): powerbi # set dataset_type_mapping to all supported data platform #7598
Closed
siddiquebagwan
wants to merge
14
commits into
datahub-project:master
from
siddiquebagwan:master+default-dataset-mapping
Closed
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
ca2cb10
dataset_type_mapping is set to all supported dataplatform
siddiquebagwan-gslab 4003892
Merge branch 'master' into master+default-dataset-mapping
siddiquebagwan-gslab 7f30c16
Merge branch 'master' into master+default-dataset-mapping
siddiquebagwan 5e868f7
Merge branch 'master' into master+default-dataset-mapping
siddiquebagwan a37c76d
Merge branch 'master' into master+default-dataset-mapping
siddiquebagwan bd7d16d
Merge branch 'master' into master+default-dataset-mapping
siddiquebagwan-gslab 49af976
deleted unwanted line
siddiquebagwan-gslab 7980c57
Merge branch 'master+default-dataset-mapping' of github.com:acryldata…
siddiquebagwan-gslab 649300e
Merge branch 'master' into master+default-dataset-mapping
siddiquebagwan 1d26572
Merge branch 'master' into master+default-dataset-mapping
siddiquebagwan e35f951
resolve merge conflict
siddiquebagwan-gslab ee6bb57
resovle merge conflict
siddiquebagwan-gslab 64ccf9d
Merge branch 'master+default-dataset-mapping' of github.com:acryldata…
siddiquebagwan-gslab 8f61323
Merge branch 'master+default-dataset-mapping' of github.com:mohdsiddi…
siddiquebagwan-gslab File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,17 @@ | ||
import logging | ||
import sys | ||
from typing import Any, Dict | ||
from typing import Any, Dict, cast | ||
from unittest import mock | ||
|
||
import pytest | ||
from freezegun import freeze_time | ||
|
||
from datahub.ingestion.run.pipeline import Pipeline | ||
from datahub.ingestion.source.powerbi.config import ( | ||
PowerBiDashboardSourceConfig, | ||
SupportedDataPlatform, | ||
) | ||
from datahub.ingestion.source.powerbi.powerbi import PowerBiDashboardSource | ||
from tests.test_helpers import mce_helpers | ||
|
||
FROZEN_TIME = "2022-02-03 07:00:00" | ||
|
@@ -651,7 +656,7 @@ def test_powerbi_ingest_urn_lower_case( | |
}, | ||
} | ||
) | ||
|
||
pipeline.config | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. deleted |
||
pipeline.run() | ||
pipeline.raise_from_status() | ||
golden_file = "golden_test_lower_case_urn_ingest.json" | ||
|
@@ -978,3 +983,50 @@ def test_workspace_container( | |
output_path=tmp_path / "powerbi_container_mces.json", | ||
golden_path=f"{test_resources_dir}/{mce_out_file}", | ||
) | ||
|
||
|
||
@freeze_time(FROZEN_TIME) | ||
@mock.patch("msal.ConfidentialClientApplication", side_effect=mock_msal_cca) | ||
@pytest.mark.integration | ||
def test_dataset_type_mapping_should_set_to_all( | ||
mock_msal, pytestconfig, tmp_path, mock_time, requests_mock | ||
): | ||
""" | ||
Here we don't need to run the pipeline. We need to verify dataset_type_mapping is set to default dataplatform | ||
""" | ||
register_mock_api(request_mock=requests_mock) | ||
|
||
new_config: dict = {**default_source_config()} | ||
|
||
del new_config["dataset_type_mapping"] | ||
|
||
pipeline = Pipeline.create( | ||
{ | ||
"run_id": "powerbi-test", | ||
"source": { | ||
"type": "powerbi", | ||
"config": { | ||
**new_config, | ||
}, | ||
}, | ||
"sink": { | ||
"type": "file", | ||
"config": { | ||
"filename": f"{tmp_path}/powerbi_lower_case_urn_mces.json", | ||
}, | ||
}, | ||
} | ||
) | ||
source_config: PowerBiDashboardSourceConfig = cast( | ||
PowerBiDashboardSource, pipeline.source | ||
).source_config | ||
assert source_config.dataset_type_mapping is not None | ||
|
||
# Generate default dataset_type_mapping and compare it with source_config.dataset_type_mapping | ||
default_dataset_type_mapping: dict = {} | ||
for item in SupportedDataPlatform: | ||
default_dataset_type_mapping[ | ||
item.value.powerbi_data_platform_name | ||
] = item.value.datahub_data_platform_name | ||
|
||
assert default_dataset_type_mapping == source_config.dataset_type_mapping |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole purpose is to make the user do LESS. Why would we include this in our sample recipe? Please reove the entire field from our starter recipe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This to give example for how to set the platform_instance with recipe, I will remove it in next PR when we make this field deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is for reference for how to set platform_instance. We can remove this field once we deprecated it in next PR.