[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-12 Thread rmetzger
GitHub user rmetzger opened a pull request: https://github.com/apache/flink/pull/301 [FLINK-1389] Allow changing the filenames of the files created when writing to a directory ... You can merge this pull request into a Git repository by running: $ git pull https://github.com/r

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-13 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-69769088 I like the idea for that. I am, though, not a big fan of configuring the output format through the config. I think there was also a consensus to move away from

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-13 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-69769143 We could also think about allowing formats to define default extensions. --- If your project is set up for it, you can reply to this email and have your reply appear o

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-14 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-69913057 Thank you for the feedback. I'll rework the pull request soon. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-16 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-70234744 BTW: How is that done in Hadoop? Can we follow a similar way, to make it easier for users to understand this? --- If your project is set up for it, you can reply to t

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-23 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71178244 Hadoops `FileOutputFormat` has the following method ```java public static String getUniqueName(JobConf conf, String name) { int partition = conf.getInt(Job

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-23 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71208053 I've updated the pull request. Please review again, I would like to merge this soon. --- If your project is set up for it, you can reply to this email and have your repl

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-23 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71262094 The change looks good, but do why do we need two parameters for 0- and 1-offset indices? I would only keep the 1-offset index (current behavior). If you think

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-24 Thread StephanEwen
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71339559 Nice mechanism, +1 I agree with Fabian, having one method should be good, or have you seen a hard requirement for 0-based counters in the file names? --- If

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-25 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71366256 The only reason why I added the two variants is that I want to give users freedom to choose between these two variants. Its one more line of code that might (someday) mak

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-25 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71366397 To be honest, I find this additional option more confusing than helpful. --- If your project is set up for it, you can reply to this email and have your reply appear on G

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-25 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71366480 Okay. Let me implement a different approach that allows users to overwrite a method to put anything they want for the file name. --- If your project is set up for it, yo

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-26 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71432009 I've completely changed the mechanism of setting a custom file name. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHu

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-26 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71433679 Hmmm, using the String pattern seems to be much more comfortable for users, no? If a user wants to have the data written out with some kind of filename pattern, s

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-26 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71434721 The user who requested this feature actually asked for a custom method to overwrite. There is one more important use-case for a custom method: If users want to h

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-26 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71435854 Well, you would have saved everybody's time of you had made this requirements clear from the beginning. Besides your first two versions didn't comply with these new requir

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-26 Thread rmetzger
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71439040 You are right, I should have made the requirements clear in the beginning. I actually had a discussion with the user which approach is the best. I took the view that a

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-26 Thread fhueske
Github user fhueske commented on the pull request: https://github.com/apache/flink/pull/301#issuecomment-71440329 No worries ;-) I understand that discussing such a "trivial" feature feels like a waste of time. Unfortunately these are the features that are easy to comment on for "ever

[GitHub] flink pull request: [FLINK-1389] Allow changing the filenames of t...

2015-01-26 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/301 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabl