[ZEPPELIN-2257] notification about incompleteness of data by tinkoff-dwh · Pull Request #2134 · apache/zeppelin

tinkoff-dwh · 2017-03-14T11:58:25Z

What is this PR for?

Added notification about exceeding the limit in the result.

What type of PR is it?

Improvement

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-2257

How should this be tested?

Create table test with more than 2 records (if not exists)
Set parameter common.max_count = 2
Execute

%jdbc
select *from test

You should see message about exceeds limit

Screenshots (if appropriate)

Questions:

Does the licenses files need update? no
Is there breaking changes for older versions? no
Does this needs documentation? no

Leemoonsoo · 2017-03-14T18:13:49Z

LGTM

Leemoonsoo · 2017-03-15T15:45:53Z

Merge to master and branch-0.7 if no further discussions.

1ambda · 2017-03-15T19:51:45Z

Sorry for late reply.

regarding to design, it would be great if we use bootstrap-alert. (warning level would be ok)
http://getbootstrap.com/components/#alerts

For example,

tinkoff-dwh · 2017-03-16T04:43:19Z

ok.

1ambda · 2017-03-16T04:47:56Z

LGTM

tinkoff-dwh · 2017-03-16T04:56:11Z

tinkoff-dwh · 2017-03-16T06:18:50Z

CI green https://travis-ci.org/tinkoff-dwh/zeppelin/builds/211619211

AhyoungRyu · 2017-03-16T06:23:24Z

@tinkoff-dwh Then you can make this CI status green by reopening-closing this PR :)

prabhjyotsingh · 2017-03-16T17:00:20Z

Just saw it, this definitely looks better. I don't have a strong opinion on this, just observed a small diff, hence, highlighting it.

This is how it gets displayed in spark/livy, with some extra information.

And this is how it will look like if there were multiple queries.

Leemoonsoo · 2017-03-17T19:01:53Z

In addition to @prabhjyotsingh mentions, there are another type of message controlled by ZEPPELIN_INTERPRETER_OUTPUT_LIMIT env variable.

All three messages Color, location, Decorations are different.
Basically these three messages are saying the same thing "Not a complete data".

So it make sense to display all three message in similar way, i think.

tinkoff-dwh · 2017-03-17T19:10:10Z

@Leemoonsoo
what final version, bootstrap?

Leemoonsoo · 2017-03-17T19:46:01Z

Message location

Because of existing message Results are limited by X. and Output exceeds 10240. Truncated. are displayed below the table.
I think showing Attention! Result is incomplete ... below the table helps keep user experience more consistent.

Message style

Bootstrap decoration is pretty. If we apply bootstrap alert to all 3 messages, then that means existing 2 message ux is changed, too.

From taking less height of the screen, receiving less attention,
To taking more height of the screen, receiving more attention.

So which is the way we should go do you guys think? @tinkoff-dwh @1ambda

Message text

All three message are different.

Results are limited by X.
Output exceeds 10240. Truncated.
Attention! Result is incomplete ...

It'll be better use similar form of message. But how they're limited are all different. And it'll be great if we user give some link how they're limited.
So for example,

Output is truncated to X rows. Learn more about common.max_count
Output is truncated to 10240 bytes. learn more about ZEPPELIN_INTERPRETER_OUTPUT_LIMIT

What do you think?

tinkoff-dwh · 2017-03-17T20:09:33Z

@Leemoonsoo
location
agree
style
Bootstrap attracts attention and this is important
text
Output is truncated to {count} {rows | bytes}. Learn more about {variable_name}

sorry for offtopic. @Leemoonsoo can you watch #2085

tinkoff-dwh · 2017-03-20T11:43:37Z

Leemoonsoo · 2017-03-23T19:44:24Z

Top margin looks bit small and bottom margin looks too large. What do you think adjust them little bit and make top and bottom margin the same?

@1ambda @AhyoungRyu @prabhjyotsingh @felixcheung @zjffdu @jongyoul Can you guys take a look and give some feedback how this new message style looks like?

tinkoff-dwh · 2017-03-23T20:17:42Z

@Leemoonsoo
I think now is fine (data table + message separated from footer), but I can increase the top margin

Leemoonsoo · 2017-03-23T20:32:28Z

@tinkoff-dwh thanks!

It's nit, but bottom margin is bit much compare to other margins in paragraph. i tried mark other margins.

Of course there's no rule that all the margin should be the same, but the margin on bottom looks bit inconsistent. Message is already decorated by bootstrap box so it already gives good visual separation.

What do you think?

tinkoff-dwh · 2017-03-23T20:48:14Z

@Leemoonsoo
possible. This margin within HTML Message maybe there are problems in other messages(HTML, without bootstrap). I'll try.

Leemoonsoo · 2017-03-23T21:18:25Z

@tinkoff-dwh Ah, i see. i didn't see where this margin comes from.

AhyoungRyu · 2017-03-24T03:22:46Z

Tested and basically agree with @Leemoonsoo. But one thing I just noticed is this new message can be seen in report mode.

I think it would be better we can hide this msg in this case.

tinkoff-dwh · 2017-03-24T05:27:02Z

@AhyoungRyu
if you configure the note and working only in report mode then you will not see when the report is incomplete.I can add the close button

AhyoungRyu · 2017-03-24T05:59:43Z

I can add the close button

@tinkoff-dwh Yeah that's better. Having close button will be better for other cases as well.

tinkoff-dwh · 2017-03-24T06:20:51Z

AhyoungRyu · 2017-03-24T06:31:35Z

@tinkoff-dwh Thanks! Looks good :)

tinkoff-dwh · 2017-03-27T06:26:31Z

@Leemoonsoo
Yes you were right, this margin from alert (bootstrap class). fix it

tinkoff-dwh · 2017-03-29T05:48:20Z

Ready to review

sotnich · 2017-03-29T12:34:38Z

LGTM

# Conflicts: # livy/src/main/java/org/apache/zeppelin/livy/LivySparkSQLInterpreter.java

Leemoonsoo · 2017-04-04T19:55:18Z

LGTM and merge to master if no further discussions.

Thanks @tinkoff-dwh for the improvement!

### What is this PR for? Added notification about exceeding the limit in the result. ### What type of PR is it? Improvement ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-2257 ### How should this be tested? 1. Create table *test* with more than 2 records (if not exists) 2. Set parameter `common.max_count ` = 2 3. Execute ``` %jdbc select *from test ``` You should see message about exceeds limit ### Screenshots (if appropriate) ![6](https://cloud.githubusercontent.com/assets/25951039/23899435/f5a23e48-08d6-11e7-9cb0-1613398ce22e.png) ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: Tinkoff DWH <tinkoff.dwh@gmail.com> Closes apache#2134 from tinkoff-dwh/ZEPPELIN-2257 and squashes the following commits: 367ac65 [Tinkoff DWH] [ZEPPELIN-2257] merge 1048214 [Tinkoff DWH] Merge remote-tracking branch 'origin/master' into ZEPPELIN-2257 3c52b52 [Tinkoff DWH] [ZEPPELIN-2257] custom css class for alert 2e6c976 [Tinkoff DWH] [ZEPPELIN-2257] close button to alert d6dbe3e [Tinkoff DWH] Merge remote-tracking branch 'origin/master' into ZEPPELIN-2257 edeca0e [Tinkoff DWH] [ZEPPELIN-2257] notifications about incompleteness of data

tinkoff-dwh closed this Mar 16, 2017

tinkoff-dwh reopened this Mar 16, 2017

tinkoff-dwh force-pushed the ZEPPELIN-2257 branch from 318083c to cd0e01e Compare March 20, 2017 18:23

[ZEPPELIN-2257] notifications about incompleteness of data

edeca0e

tinkoff-dwh force-pushed the ZEPPELIN-2257 branch from 22a2c25 to edeca0e Compare March 20, 2017 19:09

Merge remote-tracking branch 'origin/master' into ZEPPELIN-2257

d6dbe3e

tinkoff-dwh closed this Mar 20, 2017

tinkoff-dwh reopened this Mar 20, 2017

tinkoff-dwh closed this Mar 21, 2017

tinkoff-dwh reopened this Mar 21, 2017

[ZEPPELIN-2257] close button to alert

2e6c976

[ZEPPELIN-2257] custom css class for alert

3c52b52

tinkoff-dwh force-pushed the ZEPPELIN-2257 branch from 1a1366c to 3c52b52 Compare March 27, 2017 06:38

tinkoff-dwh closed this Mar 27, 2017

tinkoff-dwh reopened this Mar 27, 2017

tinkoff-dwh added 2 commits April 3, 2017 15:30

Merge remote-tracking branch 'origin/master' into ZEPPELIN-2257

1048214

# Conflicts: # livy/src/main/java/org/apache/zeppelin/livy/LivySparkSQLInterpreter.java

[ZEPPELIN-2257] merge

367ac65

asfgit closed this in 61b7162 Apr 5, 2017

tinkoff-dwh deleted the ZEPPELIN-2257 branch April 6, 2017 05:06

Conversation

tinkoff-dwh commented Mar 14, 2017

What is this PR for?

What type of PR is it?

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

Uh oh!

Leemoonsoo commented Mar 14, 2017

Uh oh!

Leemoonsoo commented Mar 15, 2017

Uh oh!

1ambda commented Mar 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tinkoff-dwh commented Mar 16, 2017

Uh oh!

1ambda commented Mar 16, 2017

Uh oh!

tinkoff-dwh commented Mar 16, 2017

Uh oh!

tinkoff-dwh commented Mar 16, 2017

Uh oh!

AhyoungRyu commented Mar 16, 2017

Uh oh!

prabhjyotsingh commented Mar 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Leemoonsoo commented Mar 17, 2017

Uh oh!

tinkoff-dwh commented Mar 17, 2017

Uh oh!

Leemoonsoo commented Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Message location

Message style

Message text

Uh oh!

tinkoff-dwh commented Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tinkoff-dwh commented Mar 20, 2017

Uh oh!

Leemoonsoo commented Mar 23, 2017

Uh oh!

tinkoff-dwh commented Mar 23, 2017

Uh oh!

Leemoonsoo commented Mar 23, 2017

Uh oh!

tinkoff-dwh commented Mar 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Leemoonsoo commented Mar 23, 2017

Uh oh!

AhyoungRyu commented Mar 24, 2017

Uh oh!

tinkoff-dwh commented Mar 24, 2017

Uh oh!

AhyoungRyu commented Mar 24, 2017

Uh oh!

tinkoff-dwh commented Mar 24, 2017

Uh oh!

AhyoungRyu commented Mar 24, 2017

Uh oh!

tinkoff-dwh commented Mar 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tinkoff-dwh commented Mar 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sotnich commented Mar 29, 2017

Uh oh!

Leemoonsoo commented Apr 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

1ambda commented Mar 15, 2017 •

edited

Loading

prabhjyotsingh commented Mar 16, 2017 •

edited

Loading

Leemoonsoo commented Mar 17, 2017 •

edited

Loading

tinkoff-dwh commented Mar 17, 2017 •

edited

Loading

tinkoff-dwh commented Mar 23, 2017 •

edited

Loading

tinkoff-dwh commented Mar 27, 2017 •

edited

Loading

tinkoff-dwh commented Mar 29, 2017 •

edited

Loading