Skip to content

Conversation

@minahlee
Copy link
Member

@minahlee minahlee commented Jun 13, 2016

What is this PR for?

Add csv download from front-end leveraging #714

What type of PR is it?

Improvement

What is the Jira issue?

ZEPPELIN-997

Screenshots (if appropriate)

Before
screen shot 2016-06-13 at 4 55 02 pm

After
screen shot 2016-06-14 at 12 42 01 am

screen shot 2016-06-14 at 12 41 49 am

### Questions: - Does the licenses files need update? No - Is there breaking changes for older versions? No - Does this needs documentation? No

@corneadoug
Copy link
Contributor

Could we use a separator between the button and the carret?
http://getbootstrap.com/components/#btn-dropdowns-split

Default download could be CSV

@minahlee
Copy link
Member Author

@corneadoug I just changed, please check and let me know your opinion.

</div>
<span>
<button type="button" class="btn btn-default btn-sm" style="margin-left:10px"
tooltip="Download Data as TSV" tooltip-placement="bottom"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you restore the tooltip on the main button? (with CSV instead of TSV)

@corneadoug
Copy link
Contributor

Unrelated CI failure (timeout)
LGTM

SaveAsService.SaveAs(tsv, 'data', 'tsv');
var extension = '';
if (delimiter === '\t') {
extension = 'tsv';
Copy link
Member

Choose a reason for hiding this comment

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

this is likely out of scope for this change, for csv (likely tsv), strictly speaking we would need to escape any , in the data:
https://en.wikipedia.org/wiki/Comma-separated_values#General_functionality
otherwise when the data is read it will be misaligned.

@minahlee
Copy link
Member Author

@felixcheung Thanks for coming up with the issue, let me handle it in another PR. I created new issue for that.

@Leemoonsoo
Copy link
Member

LGTM

@corneadoug
Copy link
Contributor

Merging in Master if there is no more discussions

@asfgit asfgit closed this in 048e432 Jun 16, 2016
asfgit pushed a commit that referenced this pull request Jun 22, 2016
Closes #6 (fixed by #1008)
Closes #89 (fixed by #1008)
Closes #46 (fixed by #140)
Closes #60 (fixed by #361)
Closes #211 (fixed by #361)
Closes #100 (fixed by #114)
Closes #190 (fixed by #796)
Closes #527
@minahlee minahlee deleted the ZEPPELIN-997 branch June 23, 2016 05:46
asfgit pushed a commit that referenced this pull request Jun 25, 2016
### What is this PR for?
Add csv download from front-end leveraging #714

### What type of PR is it?
Improvement

### What is the Jira issue?
[ZEPPELIN-997](https://issues.apache.org/jira/browse/ZEPPELIN-997)

### Screenshots (if appropriate)
**Before**
<img width="347" alt="screen shot 2016-06-13 at 4 55 02 pm" src="https://cloud.githubusercontent.com/assets/8503346/16027024/acb80824-3187-11e6-8535-090b06e4807e.png">

**After**
<img width="354" alt="screen shot 2016-06-14 at 12 42 01 am" src="https://cloud.githubusercontent.com/assets/8503346/16034849/dd6f48c4-31c8-11e6-92b8-809b3f27d429.png">

<img width="357" alt="screen shot 2016-06-14 at 12 41 49 am" src="https://cloud.githubusercontent.com/assets/8503346/16034844/da6cef46-31c8-11e6-8ef2-d11c460eaa02.png">

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: Mina Lee <[email protected]>

Closes #1008 from minahlee/ZEPPELIN-997 and squashes the following commits:

dabb603 [Mina Lee] Add tooltip and change style for dropdown button
e48c303 [Mina Lee] Fix integration test after adding csv download button
5437c4f [Mina Lee] Use split button dropdowns for downloading data
2ad6f47 [Mina Lee] Export data to csv

(cherry picked from commit 048e432)
Signed-off-by: Mina Lee <[email protected]>
@qinzl1
Copy link

qinzl1 commented Aug 23, 2016

if i can export more than 1000 rows

@corneadoug
Copy link
Contributor

@qinzl1 It might be independent from the download button.
Queries are usually limited to 1000 rows (it is set in the interpreter's setting).
Can you check that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants