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(ui/data-contract): Data contract UI under Validation Tab #10625

Merged
merged 32 commits into from
Jun 19, 2024

Conversation

amit-apptware
Copy link
Contributor

@amit-apptware amit-apptware commented May 31, 2024

Screencast.from.14-06-24.04.42.06.PM.IST.webm

Test cases

Tested with below scenarios on OSS for Data Contract Feature:

  1. Checked with assertion type Dataset, Volume -> combined in Data Quality
  2. Checked with assertion type Freshness -> it is showing as Freshness section on Data Contract Tab
  3. Assigned Data contract symbol to assertion list as like SaaS does
  4. Data contract symbol is redirecting correctly to Data Contract Tab
  5. Adding Contract and Removing contract also working Properly

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

Tested Below Scenarios:

  • Checked with assertion type Dataset, Volume -> combined in Data Quality
  • Checked with assertion type Freshness -> it is showing as Freshness section on Data Contract Tab
  • Assigned Data contract symbol to assertion list as like SaaS does
  • Data contract symbol is redirecting correctly to Data Contract Tab
  • Adding Contract and Removing contract also working Properly

@github-actions github-actions bot added product PR or Issue related to the DataHub UI/UX community-contribution PR or Issue raised by member(s) of DataHub Community labels May 31, 2024
@jayacryl jayacryl changed the title feat(ui/data-contract) : Add Data contract to Validation Tab feat(ui/data-contract): Data contract to Validation Tab Jun 13, 2024
@jayacryl jayacryl changed the title feat(ui/data-contract): Data contract to Validation Tab feat(ui/data-contract): Data contract UI under Validation Tab Jun 13, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice, for context john - Amit did a minimal set of changes to make the existing assertion list in OSS selectable (check/uncheck option). This will work for OSS until we get to porting over our acryl assertion list code.

@@ -77,6 +81,26 @@ export const ValidationsTab = () => {
},
];

if (appConfig.config.featureFlags?.dataContractsEnabled) {
// If contracts feature is enabled, add to list.

Copy link
Collaborator

Choose a reason for hiding this comment

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

super nit we can remove space

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

Comment on lines 25 to 103
const getPropertyFromVolumeType = (type: VolumeAssertionType) => {
switch (type) {
case VolumeAssertionType.RowCountTotal:
return 'rowCountTotal' as VolumeTypeField;
case VolumeAssertionType.RowCountChange:
return 'rowCountChange' as VolumeTypeField;
case VolumeAssertionType.IncrementingSegmentRowCountTotal:
return 'incrementingSegmentRowCountTotal' as VolumeTypeField;
case VolumeAssertionType.IncrementingSegmentRowCountChange:
return 'incrementingSegmentRowCountChange' as VolumeTypeField;
default:
throw new Error(`Unknown volume assertion type: ${type}`);
}
};

const getVolumeTypeInfo = (volumeAssertion: VolumeAssertionInfo) => {
const result = volumeAssertion[getPropertyFromVolumeType(volumeAssertion.type)];
if (!result) {
return undefined;
}
return result;
};

const getIsRowCountChange = (type: VolumeAssertionType) => {
return [VolumeAssertionType.RowCountChange, VolumeAssertionType.IncrementingSegmentRowCountChange].includes(type);
};

const getVolumeTypeDescription = (volumeType: VolumeAssertionType) => {
switch (volumeType) {
case VolumeAssertionType.RowCountTotal:
case VolumeAssertionType.IncrementingSegmentRowCountTotal:
return 'has';
case VolumeAssertionType.RowCountChange:
case VolumeAssertionType.IncrementingSegmentRowCountChange:
return 'should grow by';
default:
throw new Error(`Unknown volume type ${volumeType}`);
}
};

const getOperatorDescription = (operator: AssertionStdOperator) => {
switch (operator) {
case AssertionStdOperator.GreaterThanOrEqualTo:
return 'at least';
case AssertionStdOperator.LessThanOrEqualTo:
return 'at most';
case AssertionStdOperator.Between:
return 'between';
default:
throw new Error(`Unknown operator ${operator}`);
}
};

const getValueChangeTypeDescription = (valueChangeType: AssertionValueChangeType) => {
switch (valueChangeType) {
case AssertionValueChangeType.Absolute:
return 'rows';
case AssertionValueChangeType.Percentage:
return '%';
default:
throw new Error(`Unknown value change type ${valueChangeType}`);
}
};

const getParameterDescription = (parameters: AssertionStdParameters) => {
if (parameters.value) {
return formatNumberWithoutAbbreviation(
parseMaybeStringAsFloatOrDefault(parameters.value.value, parameters.value.value),
);
}
if (parameters.minValue && parameters.maxValue) {
return `${formatNumberWithoutAbbreviation(
parseMaybeStringAsFloatOrDefault(parameters.minValue.value, parameters.minValue.value),
)} and ${formatNumberWithoutAbbreviation(
parseMaybeStringAsFloatOrDefault(parameters.maxValue.value, parameters.maxValue.value),
)}`;
}
throw new Error('Invalid assertion parameters provided');
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since it's a lot of utils before the component code begins I wonder if it's worth moving this to a utils file.
And I know in SaaS we have many of these under the assertion/builder/... folder. So we should modify SaaS to take it out of that assertion/builder/... folder utils file and put it in a higher level common folder.

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.

undo all changes in lock file

@jayacryl
Copy link
Collaborator

Looks like there's failing checks, please fix before merge.

@jayacryl jayacryl merged commit 1b56035 into datahub-project:master Jun 19, 2024
38 of 39 checks passed
sleeperdeep pushed a commit to sleeperdeep/datahub that referenced this pull request Jun 25, 2024
yoonhyejin pushed a commit that referenced this pull request Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution PR or Issue raised by member(s) of DataHub Community product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants