feat(autorender): Auto render aspects that don't have frontend components in the UI #3597
Conversation
| @@ -238,6 +242,7 @@ public GmsGraphQLEngine( | |||
| this.mlPrimaryKeyType = new MLPrimaryKeyType(entityClient); | |||
| this.dataFlowType = new DataFlowType(entityClient); | |||
| this.dataJobType = new DataJobType(entityClient); | |||
| this.entityRegistry = entityRegistry; | |||
There was a problem hiding this comment.
NIT: seems odd to place this here. Can we put it below all of the types? Or above?
| @@ -213,6 +216,7 @@ public GmsGraphQLEngine( | |||
| final AnalyticsService analyticsService, | |||
| final EntityService entityService, | |||
| final RecommendationsService recommendationsService, | |||
| final EntityRegistry entityRegistry, | |||
There was a problem hiding this comment.
Can we put this at the end, instead of between 2 services?
| @AllArgsConstructor | ||
| public class WeaklyTypedAspectsResolver implements DataFetcher<CompletableFuture<List<RawAspect>>> { | ||
|
|
||
| EntityClient _aspectClient; |
There was a problem hiding this comment.
nit: shouldn't these be private finals?
| private static final JacksonDataCodec CODEC = new JacksonDataCodec(); | ||
|
|
||
| private boolean shouldReturnAspect(AspectSpec aspectSpec, AspectParams params) { | ||
| return !params.getAutoRender() || aspectSpec.isAutoRender(); |
There was a problem hiding this comment.
This is confusing on first read --
I only return an aspect if there is no "auto render" params OR if the aspect is auto-render enabled? (Not clear what the difference is, yet)
| Params to configure what list of aspects should be fetched by the aspects property | ||
| """ | ||
| input AspectParams { | ||
| autoRender: Boolean |
There was a problem hiding this comment.
Is this supposed to be for fetching ONLY those aspects which are defined as autorender?
There was a problem hiding this comment.
nit: no comment on autoRender field
| entityData?.autoRenderAspects?.map((aspect) => ({ | ||
| name: aspect.renderSpec?.displayName || aspect.aspectName, | ||
| component: () => ( | ||
| <DynamicTab |
| } | ||
|
|
||
| export default function TableValueElement({ value }: { value: any }) { | ||
| if (typeof value === 'boolean') { |
| @@ -60,6 +62,10 @@ | |||
| @Qualifier("dataHubTokenService") | |||
| private TokenService _tokenService; | |||
|
|
|||
| @Autowired | |||
| @Qualifier("entityRegistry") | |||
| private EntityRegistry _entityRegistry; | |||
There was a problem hiding this comment.
a bit sad to see another storage layer dependency ending up in GraphQL land!
| return null; | ||
| } | ||
|
|
||
| if (entity.hasAspect()) { |
There was a problem hiding this comment.
Good stuff! Is there ever a reason we'll want an "Enveloped Aspect" hre? That is the raw aspect PLUS information about when / by who it was created, the version, the system metadata, etc
There was a problem hiding this comment.
yeah- i agree that endpoint would be useful! i left space in the graphql response to add that in later 👍
entity-registry/src/main/java/com/linkedin/metadata/models/annotation/AspectAnnotation.java
Show resolved
Hide resolved
Introduced a few templates for auto rendering aspects. This allows users of Datahub to see extensions of their metadata model in the UI without updating Java or React code.
Coming soon: extend the metadata model without forking datahub!
Checklist