[WIP] feat(gms): Adds custom ownership types #7623
Conversation
pedro93
changed the title
pedro93
changed the title
github-actions
bot
added
devops
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
(I'm not from the core datahub team, the pr is shared in https://datahubspace.slack.com/archives/C02QMLWJG12/p1679611195773309)
I assume you are already planning but this change will require some additional tweaks at least on the frontend. Hope these will not be that much.
Great PR with a huge value, I'm very excited to see it on action !
| "name": "ownershipType", | ||
| "entityTypes": [ "ownershipType" ] | ||
| } | ||
| customType: optional Urn |
There was a problem hiding this comment.
WDYT about adding a default to this field, it can be "None" as default.
There was a problem hiding this comment.
Also, a name like ownershipTypeUrn would fit better in here. WDYT? Are you planning to migrate current owner definitions to the new one in this pr?
There was a problem hiding this comment.
Hello @mmmeeedddsss this PR includes a DataHub Bootstrap step that will populate the database with new ownership type entity instances based on the existing non-deprecated ownership types.
There was a problem hiding this comment.
@pedro93 As far as I can see, these custom values are not used in anywhere. Do you plan to open a separate pr for the usage of these? (Such as graphql changes, frontend changes etc)
There was a problem hiding this comment.
This part is only the modelling and bootstrapping part of the feature, it was opened prematurely. I will close it and continue development in a fork. Once it is feature complete I will open the PR again.
| @Aspect = { | ||
| "name": "ownershipTypeInfo" | ||
| } | ||
| record OwnershipTypeInfo { |
There was a problem hiding this comment.
As far as I could see from other schemas, you'll need to add a URN in here too, since the types will become a dynamic asset from now on. Is that correct?
There was a problem hiding this comment.
This aspect is merely an information view of the new entity that this PR introduces. The urn is in the key aspect.
| "enableAutocomplete": true, | ||
| "boostScore": 10.0 | ||
| } | ||
| name: string |
There was a problem hiding this comment.
displayName might be a better name in here
Codecov ReportPatch coverage has no change and project coverage change:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #7623 +/- ##
==========================================
- Coverage 74.87% 67.04% -7.84%
==========================================
Files 353 353
Lines 35386 35386
==========================================
- Hits 26496 23723 -2773
- Misses 8890 11663 +2773
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 78 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
Closing PR as it is incomplete, feature development will continue in a fork and will be made available once finished. |
|
@pedro93 where can we follow the fork for this feature? |
|
The fork is private. An OSS PR will be made once the code has been developed internally. |
Summary
Adds a new entity to DataHub's metadata model graph:
OwnershipTypewhich will allow DataHub Admins to specify their own ownership types beyond and possibly in place of the existing ones:Technical Owner,Business Owner,Data Steward,NoneDetails:
Technical Owner,Business Owner,Data Steward,Noneas new entity instances of the new OwnershipType entity.INGEST_DEFAULT_OWNERSHIP_TYPES, set totrueby default which will control if the default ownership types should be ingested. If set to true, this ingestion process will only happen once.Checklist