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

[CDAP-17068] Display message when field level lineage is too large to… #12453

Merged
merged 1 commit into from
Jul 20, 2020

Conversation

elfenheart
Copy link
Contributor

… display

Either the response is greater than 50K characters, or API takes longer than 20 seconds, we will display message that Field level lineage data is too large to display.

JIRA: https://issues.cask.co/browse/CDAP-17068
Build: https://builds.cask.co/browse/CDAP-UDUT725

@elfenheart
Copy link
Contributor Author

Message:

image

@@ -250,10 +269,35 @@ class LineageSummary extends React.Component<{ classes }, ILineageState> {
</div>
);
}

if (error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

we can do

if (error) return null;
return (

...
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no we cannot. If there is error we want to display this, otherwise, there's already an actual component being rendered

// there is proper pagination for the FLL response from backend. This limit is approximately
// 1500 fields going into 1500 fields
function isFLLResponseTooLarge(response) {
const CHARACTER_LIMIT = 50000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be defined at the top of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the constant is not really applicable anywhere else

Copy link
Contributor

@yukiej yukiej left a comment

Choose a reason for hiding this comment

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

lgtm!

@elfenheart elfenheart force-pushed the feature-ui/CDAP-17068-fll-bail-out branch from 1a7a9cd to 10f56a2 Compare July 20, 2020 20:27
@elfenheart elfenheart merged commit ba38de7 into develop Jul 20, 2020
@elfenheart elfenheart deleted the feature-ui/CDAP-17068-fll-bail-out branch July 20, 2020 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants