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: Allow specifying Data Product URN via UI #9386

Merged
merged 14 commits into from
Dec 13, 2023
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,33 @@ public DataFetcherExceptionHandlerResult onException(
DataHubGraphQLErrorCode errorCode = DataHubGraphQLErrorCode.SERVER_ERROR;
String message = "An unknown error occurred.";

// note: make sure to access the true error message via `getCause()`
if (exception.getCause() instanceof IllegalArgumentException) {
IllegalArgumentException illException =
findFirstThrowableCauseOfClass(exception, IllegalArgumentException.class);
if (illException != null) {
log.error("Illegal Argument Exception");
anshbansal marked this conversation as resolved.
Show resolved Hide resolved
errorCode = DataHubGraphQLErrorCode.BAD_REQUEST;
message = exception.getCause().getMessage();
message = illException.getMessage();
}

if (exception instanceof DataHubGraphQLException) {
errorCode = ((DataHubGraphQLException) exception).errorCode();
message = exception.getMessage();
}

if (exception.getCause() instanceof DataHubGraphQLException) {
errorCode = ((DataHubGraphQLException) exception.getCause()).errorCode();
message = exception.getCause().getMessage();
DataHubGraphQLException graphQLException =
findFirstThrowableCauseOfClass(exception, DataHubGraphQLException.class);
if (graphQLException != null) {
errorCode = graphQLException.errorCode();
message = graphQLException.getMessage();
}

DataHubGraphQLError error = new DataHubGraphQLError(message, path, sourceLocation, errorCode);
return DataFetcherExceptionHandlerResult.newResult().error(error).build();
}

<T extends Throwable> T findFirstThrowableCauseOfClass(Throwable throwable, Class<T> clazz) {
while (throwable != null && throwable.getCause() != null) {
if (throwable.getCause().getClass().equals(clazz)) {
return (T) throwable.getCause();
} else {
throwable = throwable.getCause();
}
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public CompletableFuture<DataProduct> get(final DataFetchingEnvironment environm
try {
final Urn dataProductUrn =
_dataProductService.createDataProduct(
input.getId(),
input.getProperties().getName(),
input.getProperties().getDescription(),
authentication);
Expand Down
4 changes: 4 additions & 0 deletions datahub-graphql-core/src/main/resources/entity.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -11054,6 +11054,10 @@ input CreateDataProductInput {
The primary key of the Domain
"""
domainUrn: String!
"""
An id for the DataProduct
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: let's say

"An optional id for the new data product"

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

"""
id : String
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove space between id and :

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

}

"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export default function CreateDataProductModal({ domain, onCreateDataProduct, on
variables: {
input: {
domainUrn: domain.urn,
id: builderState.id || '',
Copy link
Collaborator

Choose a reason for hiding this comment

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

not || ''

this will cause an empty string id to be created.
let's just say builderState.id.

Undefined | null should be provided if an id is not

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

properties: {
name: builderState.name,
description: builderState.description || undefined,
Expand All @@ -49,10 +50,10 @@ export default function CreateDataProductModal({ domain, onCreateDataProduct, on
onClose();
}
})
.catch(() => {
.catch(( error ) => {
onClose();
message.destroy();
message.error({ content: 'Failed to create Data Product. An unexpected error occurred' });
message.error({ content: `Failed to create Data Product: ${error.message}.` });
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import React from "react";
import { Collapse, Form, Input, Typography } from "antd";
import styled from "styled-components";
import { validateCustomUrnId } from '../../../shared/textUtil';
import { DataProductBuilderFormProps } from "./types";


const FormItem = styled(Form.Item)`
.ant-form-item-label {
padding-bottom: 2px;
}
`;

const FormItemWithMargin = styled(FormItem)`
margin-bottom: 16px;
`;

const FormItemNoMargin = styled(FormItem)`
margin-bottom: 0;
`;

const AdvancedLabel = styled(Typography.Text)`
color: #373d44;
`;

export function DataProductAdvanceOption({builderState, updateBuilderState }:DataProductBuilderFormProps){
Copy link
Collaborator

Choose a reason for hiding this comment

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

naming nitpick:

Let's call this DataProductAdvancedOption

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.

lint fix: add a space between : and DataProductBuilderFormProps.

Please always pay attention to the small details!

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


function updateDataProductId(id: string) {
updateBuilderState({
...builderState,
id,
});
}

return (
<Collapse ghost>
<Collapse.Panel header={<AdvancedLabel>Advanced Options</AdvancedLabel>} key="1">
<FormItemWithMargin
label={<Typography.Text strong>Data Product Id</Typography.Text>}
help="By default, a random UUID will be generated to uniquely identify this data product. If
you'd like to provide a custom id instead to more easily keep track of this data product,
you may provide it here. Be careful, you cannot easily change the data product id after
creation."
>
<FormItemNoMargin
rules={[
() => ({
validator(_, value) {
if (value && validateCustomUrnId(value)) {
return Promise.resolve();
}
return Promise.reject(new Error('Please enter a valid Data product id'));
},
}),
]}
>
<Input
data-testid="data-product-id"
placeholder="engineering"
value={builderState.id}
onChange={(e) => updateDataProductId(e.target.value)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

overall, nice component!

/>
</FormItemNoMargin>
</FormItemWithMargin>
</Collapse.Panel>
</Collapse>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,14 @@ import React from 'react';
import styled from 'styled-components';
import { Editor as MarkdownEditor } from '../../shared/tabs/Documentation/components/editor/Editor';
import { ANTD_GRAY } from '../../shared/constants';
import { DataProductBuilderState } from './types';
import { DataProductBuilderFormProps } from './types';
import { DataProductAdvanceOption } from './DataProductAdvanceOption';

const StyledEditor = styled(MarkdownEditor)`
border: 1px solid ${ANTD_GRAY[4]};
`;

type Props = {
builderState: DataProductBuilderState;
updateBuilderState: (newState: DataProductBuilderState) => void;
};

export default function DataProductBuilderForm({ builderState, updateBuilderState }: Props) {
export default function DataProductBuilderForm({ builderState, updateBuilderState }: DataProductBuilderFormProps) {
function updateName(name: string) {
updateBuilderState({
...builderState,
Expand Down Expand Up @@ -47,6 +43,7 @@ export default function DataProductBuilderForm({ builderState, updateBuilderStat
<Form.Item label={<Typography.Text strong>Description</Typography.Text>}>
<StyledEditor doNotFocus content={builderState.description} onChange={updateDescription} />
</Form.Item>
<DataProductAdvanceOption builderState={builderState} updateBuilderState={updateBuilderState}/>
</Form>
);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
export type DataProductBuilderState = {
name: string;
id?: string;
description?: string;
};

export type DataProductBuilderFormProps = {
builderState: DataProductBuilderState;
updateBuilderState: (newState: DataProductBuilderState) => void;
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.linkedin.metadata.service;

import static com.linkedin.metadata.Constants.DATA_PRODUCT_ENTITY_NAME;

import com.datahub.authentication.Authentication;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
Expand All @@ -22,6 +24,7 @@
import com.linkedin.metadata.graph.GraphClient;
import com.linkedin.metadata.query.filter.RelationshipDirection;
import com.linkedin.metadata.utils.EntityKeyUtils;
import com.linkedin.r2.RemoteInvocationException;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
Expand Down Expand Up @@ -58,11 +61,26 @@ public DataProductService(@Nonnull EntityClient entityClient, @Nonnull GraphClie
* @return the urn of the newly created DataProduct
*/
public Urn createDataProduct(
@Nullable String name, @Nullable String description, @Nonnull Authentication authentication) {
@Nullable String id,
@Nullable String name,
@Nullable String description,
@Nonnull Authentication authentication) {

// 1. Generate a unique id for the new DataProduct.
final DataProductKey key = new DataProductKey();
key.setId(UUID.randomUUID().toString());
if (id != null && !id.isBlank()) {
key.setId(id);
anshbansal marked this conversation as resolved.
Show resolved Hide resolved
} else {
key.setId(UUID.randomUUID().toString());
}
try {
if (_entityClient.exists(
EntityKeyUtils.convertEntityKeyToUrn(key, DATA_PRODUCT_ENTITY_NAME), authentication)) {
throw new IllegalArgumentException("This Data product already exists!");
}
} catch (RemoteInvocationException e) {
throw new RuntimeException("Unable to check for existence of Data Product!");
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's include the underlying context in this exception:

throw new RuntimeException("Unable to check for existence of Data Product!", e)

}

// 2. Create a new instance of DataProductProperties
final DataProductProperties properties = new DataProductProperties();
Expand Down
Loading