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.

Sign up for GitHub

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

Jump to bottom

fix(validation) Fail validation error silently instead of crashing #5314

Merged
merged 2 commits into from
Jul 1, 2022
Merged

fix(validation) Fail validation error silently instead of crashing #5314

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c3b193
fix(validation) Fail validation error silently instead of crashing
Jul 1, 2022
a0e394e
handle other edge cases
Jul 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions ...-web-react/src/app/entity/shared/tabs/Dataset/Validations/DatasetAssertionDescription.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,88 +74,90 @@ const getColumnAggregationText = (
aggregation: AssertionStdAggregation | undefined | null,
field: SchemaFieldRef | undefined,
) => {
let columnText = field?.path;
if (field === undefined) {
throw new Error(`Invalid field provided for Dataset Assertion with scope Column ${JSON.stringify(field)}`);
columnText = 'undefined';
console.error(`Invalid field provided for Dataset Assertion with scope Column ${JSON.stringify(field)}`);
}
switch (aggregation) {
// Hybrid Aggregations
case AssertionStdAggregation.UniqueCount: {
return (
<Typography.Text>
Unique value count for column <Typography.Text strong>{field.path}</Typography.Text> is
Unique value count for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
case AssertionStdAggregation.UniquePropotion: {
return (
<Typography.Text>
Unique value proportion for column <Typography.Text strong>{field.path}</Typography.Text> is
Unique value proportion for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
case AssertionStdAggregation.NullCount: {
return (
<Typography.Text>
Null count for column <Typography.Text strong>{field.path}</Typography.Text> is
Null count for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
case AssertionStdAggregation.NullProportion: {
return (
<Typography.Text>
Null proportion for column <Typography.Text strong>{field.path}</Typography.Text> is
Null proportion for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
// Numeric Aggregations
case AssertionStdAggregation.Min: {
return (
<Typography.Text>
Minimum value for column <Typography.Text strong>{field.path}</Typography.Text> is
Minimum value for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
case AssertionStdAggregation.Max: {
return (
<Typography.Text>
Maximum value for column <Typography.Text strong>{field.path}</Typography.Text> is
Maximum value for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
case AssertionStdAggregation.Mean: {
return (
<Typography.Text>
Mean value for column <Typography.Text strong>{field.path}</Typography.Text> is
Mean value for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
case AssertionStdAggregation.Median: {
return (
<Typography.Text>
Median value for column <Typography.Text strong>{field.path}</Typography.Text> is
Median value for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
case AssertionStdAggregation.Stddev: {
return (
<Typography.Text>
Standard deviation for column <Typography.Text strong>{field.path}</Typography.Text> is
Standard deviation for column <Typography.Text strong>{columnText}</Typography.Text> is
</Typography.Text>
);
}
// Native Aggregations
case AssertionStdAggregation.Native: {
return (
<Typography.Text>
Column <Typography.Text strong>{field.path}</Typography.Text> values are
Column <Typography.Text strong>{columnText}</Typography.Text> values are
</Typography.Text>
);
}
default:
// No aggregation on the column at hand. Treat the column as a set of values.
return (
<Typography.Text>
Column <Typography.Text strong>{field.path}</Typography.Text> values are
Column <Typography.Text strong>{columnText}</Typography.Text> values are
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to handle edge case on line 180 also?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or line 65

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or line 48

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh no I don't believe so. If fields is undefined, we pass in undefined same thing if fields is an empty list (the situation we were seeing with some users)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh wait I didn't see your other two comments (github was stale)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

but yes going to handle those other edge cases where we throw

</Typography.Text>
);
}
Expand Down