Skip to content

[ZEPPELIN-1789] Make exported data file name more explicit#1822

Closed
AhyoungRyu wants to merge 3 commits intoapache:masterfrom
AhyoungRyu:ZEPPELIN-1789
Closed

[ZEPPELIN-1789] Make exported data file name more explicit#1822
AhyoungRyu wants to merge 3 commits intoapache:masterfrom
AhyoungRyu:ZEPPELIN-1789

Conversation

@AhyoungRyu
Copy link
Contributor

@AhyoungRyu AhyoungRyu commented Dec 30, 2016

What is this PR for?

Currently we can download table data set as .csv or .tsv format in Zeppelin note. But since the exported file name is always set as data.csv, it will be difficult to distinguish the each file if user tries to export it several times. So I changed the naming rule like below.

  1. if the paragraph has title e.g. this is title, then the exported csv file name will be
    this is title_YYYY-MM-DD hh:mm:ss a.csv .

  2. if not, the file name will be data_YYYY-MM-DD hh:mm:ss a.csv .

Please see the short discussion in ZEPPELIN-1789 for the more details. I just made this PR by following one person's opinion who raised this issue (Ruslan Dautkhanov ). So if anyone has better idea, please feel free to share :)

What type of PR is it?

Improvement

What is the Jira issue?

ZEPPELIN-1789

How should this be tested?

  1. run Zeppelin web dev mode under zeppelin-web
$ npm run dev
  1. browse http://localhost:9000 and go to Zeppelin-Spark tutorial note
  2. click screen shot 2016-12-30 at 2 26 32 pm

If the paragraph has title, then the exported file name will be "paragraph_title_YYYY-MM-DD hh:mm:ss a".csv, if not, will be "data_YYYY-MM-DD hh:mm:ss a".csv.

The timestamp is not current time but paragraph finished time

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
Copy link
Member

Thanks @AhyoungRyu for the improvement.

How about use dateFinished instead of current time in the file name?

@AhyoungRyu
Copy link
Contributor Author

@Leemoonsoo Sounds good. Let me update then :)

@AhyoungRyu
Copy link
Contributor Author

@Leemoonsoo Addressed your comment in d7d0087. Please check if you have time.

$scope.exportToDSV = function(delimiter) {
var dsv = '';
var dateFinished = moment(paragraph.dateFinished).format('YYYY-MM-DD hh:mm:ss A');
var exportedFileName = paragraph.title ? paragraph.title + '_' + dateFinished : 'Exported data_' + dateFinished;
Copy link
Member

Choose a reason for hiding this comment

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

nit: shell we make file name starts with lower case?
data_ instead of Exported data_, what do you think?

@AhyoungRyu
Copy link
Contributor Author

@Leemoonsoo I've just updated as you said and the PR description as well. Please take a look :)

@Leemoonsoo
Copy link
Member

@AhyoungRyu Thanks~ LGTM!

@AhyoungRyu
Copy link
Contributor Author

CI failure is not related this issue


Results :

Failed tests: 
  NotebookTest.testAbortParagraphStatusOnInterpreterRestart:760 expected:<ABORT> but was:<RUNNING>

Tests run: 150, Failures: 1, Errors: 0, Skipped: 0

it's reported in ZEPPELIN-1857.

Merge if there are no more comments

@asfgit asfgit closed this in 9b430de Jan 5, 2017
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.

2 participants