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

feat(ingestion/powerbi): Ingest datasets not used in PowerBI visualization(tiles/pages) #8212

Conversation

siddiquebagwan
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the ingestion PR or Issue related to the ingestion of metadata label Jun 10, 2023
@siddiquebagwan siddiquebagwan changed the title feat(ingestion/powerbi): Ingest datasets not used in any PowerBI visualization(tiles/pages) feat(ingestion/powerbi): Ingest datasets not used in PowerBI visualization(tiles/pages) Jun 10, 2023
Copy link
Collaborator

@hsheth2 hsheth2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly looking good, just a minor code cleanup thing

)
cur_workspace.scan_result = workspace_metadata
cur_workspace.datasets = self._get_workspace_datasets(
cur_workspace.scan_result
)
for value in cur_workspace.datasets.values():
logger.info(f"Mohd = {value.name}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

dataset_workunits = self.mapper.generate_container_for_dataset(dataset)
for workunit in dataset_workunits:
yield workunit
yield from self.extract_datasets_as_containers()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd like to keep the self.source_config.extract_datasets_to_containers check here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these tests are getting pretty unwieldy - is there a way we can reduce the duplication across all the golden files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remove the unwanted mock data. Now this file size got reduced to 2k

… github.com:acryldata/datahub-fork into master+acr-5435-ingest-independent-datasets-powerbi
@hsheth2
Copy link
Collaborator

hsheth2 commented Jun 15, 2023

Merging because these changes aren't related to the kafka docker build failure

@hsheth2 hsheth2 merged commit e7e07a7 into datahub-project:master Jun 15, 2023
40 of 41 checks passed
tusharm pushed a commit to tusharm/datahub that referenced this pull request Jun 20, 2023
…ation(tiles/pages) (datahub-project#8212)

Co-authored-by: MohdSiddiqueBagwan <mohdsiddique.bagwan@gslab.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ingestion PR or Issue related to the ingestion of metadata
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants