[ZEPPELIN-3511] remove old button "Download Data as CSV/TSV"

[sanjaydasgupta]

What is this PR for?

This PR removes the legacy method (button and dropdown near the top left of each paragraph) for exporting the displayed data as a CSV or TSV file. The widgets of the legacy method are seen in the images below.

The legacy method is now redundant as a new improved data-export menu is available (top right corner of data display grid). The new method also does not suffer from certain issues in the legacy method. The new method is seen in the image below.

What type of PR is it?

[Improvement]

Todos

What is the Jira issue?

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

How should this be tested?

1. Travis CI pass
2. Manual checking (see screenshot below)

Screenshots (if appropriate)

Questions:

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

[jongyoul]

LGTM, @prabhjyotsingh Could you please verify it finally? I'll merge it there's no more discussion.

[prabhjyotsingh]

Tested this PR locally on FF, Safari, Chrome, and Edge25 works well.

The only concern over here is which one should we be removing the "legacy method" or the one that is avail with the data-grid table. I mean the "legacy method" is always shown even when the visualization is selected, and the download button on data-grid get hidden as soon as visualization.

If we all agree to this then LGTM.

[sanjaydasgupta]

Thanks for the input @prabhjyotsingh.

Allow me to list the advantages of both methods, and to suggest a possible way forward for your consideration.

1. The legacy method has two big advantages: a) It is well known, so making it disappear without notice will be a very unpleasant jolt to all existing users. b) Being always available, it is usable when either the grid or any visualization is displayed.

2. The ui-grid menu approach has the following advantages: a) It does not suffer from the problems mentioned in the issue - though one of them has been fixed. b) It allows downloading to an Excel file as well. c) Being based on a 3rd party package, the support effort will be less, and periodic enhancements can be expected automatically.

Considering everything, it would seem that the advantage in 1(a) will be the main loss. The loss of 1(b) will become acceptable once users are familiar with the new method, as users can switch to the grid to download data.

If we agree to move progressively towards the ui-grid menu approach (without confusing users who are familiar with the legacy method) the following could be an option:

In place of the legacy method button and dropdown we add a label with a tool-tip popup that informs the user where the data-download method has moved. This informative label can be maintained for one or two releases, by when all users will be familiar with the new menu. The label can then be removed.

[sanjaydasgupta]

Please see my suggestion (described above) implemented in the image below. The legacy method's button (with the very familiar download icon) has been retained with a different tool-tip message, and clicking on the button has a different effect - switches to the grid view. If this button is retained for 1 or 2 releases, it will help existing users to discover and transition to the new ui-grid menu.

[jongyoul]

@sanjaydasgupta I'm not sure your option is better than noticing with a release message. Generally, we add breaking change message like this kind of modification. Personally, I like to remove this button and use grid's download, mainly because of 2.c.

[prabhjyotsingh]

Sure, looks good.

[sanjaydasgupta]

Thanks @prabhjyotsingh.

I will wait for a clear common view before making any further changes.

[Tagar]

Was thinking of another solution.. is it possible to move grid-ui's

• Export all/visible data as csv/xls

four options to that old Download menu?

So grid-ui would only have more rare-used "Columns: .. " items, and export items will be moves completely to that older button that users got used to.

So old button would exposes newer angular-ui-grid export functionality. Thoughts?

[sanjaydasgupta]

Thanks for the ideas @Tagar

If we focus on long term advantages and stability, then continuing to maintain the old button will be a disadvantage. If the old button is retained, it will have to be revisited (implement, test, document) every time the ui-grid menu adds a new feature or changes an existing feature. This is clearly not desirable, and should not be our choice of the way forward.

I had offered to provide a transition mechanism based on a visually similar button, but just as a transition mechanism - to be removed in 1 or 2 releases.

The ui-grid already has some nice features (like PDF output) that zeppelin users have requested, and going with ui-grid will IMHO be a good decision.

[Tagar]

@sanjaydasgupta I agree with what you're saying but what I was suggesting is just hook up old button to ui-grid

[jongyoul]

I agree that the hook up ui-grid from old button makes sense. @Tagar @sanjaydasgupta

[sanjaydasgupta]

@jongyoul, @Tagar, could you please confirm my understanding of your suggestions below:

1. The old button should not be removed. It should retain its visual form and function (although the implementation of its functions may change)
2. The CSV and TSV saving functions of the old button should reuse and invoke corresponding functions of the angular ui-grid.
3. To implement the TSV part of (2) two extra items (Export all data as tsv, and Export visible data as tsv) should be added to ui-grid's menu item.

If your answer to most of the above points is yes, we should close this PR as it has traveled a long way down the wrong path :-) But before we do that let me request you to review once more the motivations in the JIRA (ZEPPELIN-3511) and the long term maintainability benefits of removing the old button (also emphasized here).

Thank you for your attention.

[Tagar]

my 2 cents: 1. yes. 2,3. yes. except that grid-ui doesn't have native tsv export, right? I thought grid-ui only allows to export as csv and as xlsx, is this correct? If it is, we might just drop tsv export. I personally never used it. csv and xlsx exports through grid-ui should be enough.

[jongyoul]

@sanjaydasgupta As a longterm plan, I like to remove old button as it's not maintained at all for now. But discussing those kinds of issues in a PR is not proper. How about moving this discussion on dev@? We would remove old download button if we could make a consensus on the mailing list.

BTW, personally, I give my 2 cents to remove old button. :-)

[sanjaydasgupta]

@jongyoul I agree.

Will return to this in a day or two.

[jongyoul]

ping @sanjaydasgupta

[sanjaydasgupta]

I have been a little busy @jongyoul, but have not forgotten it ;-)

Will be done today. Thanks.

[felixcheung]

any update?

[sanjaydasgupta]

As suggested by @jongyoul earlier, a mail was sent out to the dev list on 9th July 2018 with this subject: Should we remove "Download Data as CSV/TSV" button from version 0.8.0?

But there was very little interest from the community, with just 3 responses (including my own) -- all favoring removal of the old button.

I was not sure if 3 positive votes (and silence from a couple of others who had dissented earlier) is a strong enough indicator in favor of the motion.

Any additional guidance will help :-)

[felixcheung]

possibly? any objection from reviewers of this change/PR?

[jongyoul]

I remembered it didn't have any objection for it.

On Tue, Oct 16, 2018 at 3:57 PM Felix Cheung [email protected] wrote:

possibly? any objection from reviewers of this change/PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apache/zeppelin/pull/3013#issuecomment-430122759, or mute the thread https://github.com/notifications/unsubscribe-auth/ADcflggQ7_wES-1L6ZyPgg1NjJjvrPLnks5ulYNFgaJpZM4UhmEQ .

-- 이종열, Jongyoul Lee, 李宗烈 http://madeng.net

[felixcheung]

ok, let's go ahead then?

[jongyoul]

Sure ~

On Wed, Oct 17, 2018 at 3:22 PM Felix Cheung [email protected] wrote:

ok, let's go ahead then?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apache/zeppelin/pull/3013#issuecomment-430504087, or mute the thread https://github.com/notifications/unsubscribe-auth/ADcflkZ9hbPjuVQBEul6pFHlLhF8bJueks5ulsycgaJpZM4UhmEQ .

-- 이종열, Jongyoul Lee, 李宗烈 http://madeng.net

[sanjaydasgupta]

Allow me to remind you of the history of this PR:

1. There was an initial commit which just removed the old button and did nothing else (perhaps as intended by the author of the jira)

2. I also made another commit providing a transition proposal to guide existing users who may miss the old button

There has been a suggestion from @jongyoul that the item at (2) above is not desirable, so should I remove that piece?

Thanks for your guidance.

[jongyoul]

@sanjaydasgupta Hi, I missed this issue fully. So sorry. I think there was no objection about this issue. Correct?