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

refactor: use type mapping objects #1135

Merged
merged 11 commits into from
Dec 9, 2024
Merged

refactor: use type mapping objects #1135

merged 11 commits into from
Dec 9, 2024

Conversation

murdo-moj
Copy link
Contributor

@murdo-moj murdo-moj commented Dec 4, 2024

  • Use FindMoJDataEntityMapper objects to translate from FMD to Datahub types and subtypes
  • I chose instances of this class rather than an object to represent our entity types as there's no logic we need to group with the mappings. Also instantiating classes all over the code was messy.
  • Add FindMoJDataEntityType Enum
  • Remove EntityTypes
  • Remove SearchClient.fmd_type_to_datahub_types_mapping
  • The formation of entity_type_filters is now nicer.

Note subtype "Metric" is not included in any of our searches now

@murdo-moj murdo-moj changed the title feature: integrate type mapping objects refactor: use type mapping objects Dec 4, 2024
@murdo-moj murdo-moj marked this pull request as ready for review December 4, 2024 17:31
@murdo-moj murdo-moj requested a review from a team as a code owner December 4, 2024 17:31
home/forms/search.py Outdated Show resolved Hide resolved
MatMoore
MatMoore previously approved these changes Dec 9, 2024
Copy link
Contributor

@MatMoore MatMoore left a comment

Choose a reason for hiding this comment

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

Nice, this looks like an improvement from what we had before. Just a couple of minor suggestions.

home/service/details.py Outdated Show resolved Hide resolved
lib/datahub-client/data_platform_catalogue/entities.py Outdated Show resolved Hide resolved
Copy link
Contributor

@hjribeiro-moj hjribeiro-moj left a comment

Choose a reason for hiding this comment

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

LGTM

@murdo-moj murdo-moj merged commit d56503a into main Dec 9, 2024
10 checks passed
@murdo-moj murdo-moj deleted the fmd-1081.1-use-mappers branch December 9, 2024 11:53
Copy link

sentry-io bot commented Dec 9, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ NameError: name 'PublicationDatasetEntityMapping' is not defined /details/{result_type}/{urn} View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants