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 @@ -12,35 +12,49 @@
@Slf4j
public class DataHubDataFetcherExceptionHandler implements DataFetcherExceptionHandler {

private static final String DEFAULT_ERROR_MESSAGE = "An unknown error occurred.";

@Override
public DataFetcherExceptionHandlerResult onException(
DataFetcherExceptionHandlerParameters handlerParameters) {
Throwable exception = handlerParameters.getException();
SourceLocation sourceLocation = handlerParameters.getSourceLocation();
ResultPath path = handlerParameters.getPath();

log.error("Failed to execute DataFetcher", exception);

DataHubGraphQLErrorCode errorCode = DataHubGraphQLErrorCode.SERVER_ERROR;
String message = "An unknown error occurred.";
String message = DEFAULT_ERROR_MESSAGE;

// 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("Failed to execute", illException);
errorCode = DataHubGraphQLErrorCode.BAD_REQUEST;
message = exception.getCause().getMessage();
message = illException.getMessage();
}

if (exception instanceof DataHubGraphQLException) {
errorCode = ((DataHubGraphQLException) exception).errorCode();
message = exception.getMessage();
DataHubGraphQLException graphQLException =
findFirstThrowableCauseOfClass(exception, DataHubGraphQLException.class);
if (graphQLException != null) {
log.error("Failed to execute", graphQLException);
errorCode = graphQLException.errorCode();
message = graphQLException.getMessage();
}

if (exception.getCause() instanceof DataHubGraphQLException) {
errorCode = ((DataHubGraphQLException) exception.getCause()).errorCode();
message = exception.getCause().getMessage();
if (illException == null && graphQLException == null) {
log.error("Failed to execute", exception);
}

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) {
if (clazz.isInstance(throwable)) {
return (T) throwable;
} 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 @@ -11055,6 +11055,10 @@ input CreateDataProductInput {
The primary key of the Domain
"""
domainUrn: String!
"""
An optional id for the new data product
"""
id: String
}

"""
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,
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 DataProductAdvancedOption({builderState, updateBuilderState }: DataProductBuilderFormProps){

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)}
/>
</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 { DataProductAdvancedOption } from './DataProductAdvancedOption';

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>
<DataProductAdvancedOption 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
7 changes: 4 additions & 3 deletions smoke-test/tests/privileges/test_privileges.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def _ensure_cant_perform_action(session, json,assertion_key):
action_response.raise_for_status()
action_data = action_response.json()

assert action_data["errors"][0]["extensions"]["code"] == 403
assert action_data["errors"][0]["extensions"]["code"] == 403, action_data["errors"][0]
assert action_data["errors"][0]["extensions"]["type"] == "UNAUTHORIZED"
assert action_data["data"][assertion_key] == None

Expand Down Expand Up @@ -367,8 +367,9 @@ def test_privilege_to_create_and_manage_policies():

# Verify new user can't create a policy
create_policy = {
"query": """mutation createPolicy($input: PolicyUpdateInput!) {\n
createPolicy(input: $input) }""",
"query": """mutation createPolicy($input: PolicyUpdateInput!) {
createPolicy(input: $input)
}""",
"variables": {
"input": {
"type": "PLATFORM",
Expand Down
Loading