You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by mbalassi <gi...@git.apache.org> on 2016/02/02 14:29:02 UTC

[GitHub] flink pull request: Scala DataStream&DataStreamUtils accessors fix

GitHub user mbalassi opened a pull request:

    https://github.com/apache/flink/pull/1574

    Scala DataStream&DataStreamUtils accessors fix

    The PR contains two orthogonal approaches of fixing the access to `DataStreamUtils.collect` for a scala environment. One or both of the approaches can be merged.
    
    The first simply reexposes access to the underlying java `DataStream`, while the second effectively adds a scala API for `DataStreamUtils`.
    
    I am in favor of adding both. Usage of the latter looks as follows:
    
    ```scala
    import org.apache.flink.streaming.api.scala._
    import org.apache.flink.contrib.streaming.scala.utils._
    
    object ScalaStreamCollect {
    
      def main(args: Array[String]) {
        val env = StreamExecutionEnvironment.getExecutionEnvironment
    
        val it = env.generateSequence(0L, 10L)
          .collect()
    
        while (it.hasNext){
          print(it.next())
        }
      }
    }
    ``` 
    
    At least the first commit should be merged asap as the fix was requested on the user mailing list. [1]
    
    [1] https://mail-archives.apache.org/mod_mbox/flink-user/201602.mbox/%3CCAFqo6nQ24dtExjPOX%3DrSuWSww8skCH23Q8i7CJ3Ef5LYhDj2pA%40mail.gmail.com%3E

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mbalassi/flink scala-env-fix

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1574.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1574
    
----
commit ff728a7de3b83d9eb97a065744f0eaa5464c23ab
Author: Márton Balassi <mb...@apache.org>
Date:   2016-02-02T13:10:47Z

    [streaming] [scala] Revert removing getJavaStream() from DataStream

commit 1215c0d864e0fd2824f70b303fcac3d6a6294a6d
Author: Márton Balassi <mb...@apache.org>
Date:   2016-02-02T13:18:05Z

    [streaming] [scala] Scala wrapper for DataStreamUtils

----


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: Scala DataStream&DataStreamUtils accessors fix

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/1574


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: Scala DataStream&DataStreamUtils accessors fix

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:

    https://github.com/apache/flink/pull/1574#issuecomment-179895516
  
    Thanks, merging this...


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: Scala DataStream&DataStreamUtils accessors fix

Posted by mbalassi <gi...@git.apache.org>.
Github user mbalassi commented on the pull request:

    https://github.com/apache/flink/pull/1574#issuecomment-179434902
  
    Thanks for the comments, updated the PR accordingly.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: Scala DataStream&DataStreamUtils accessors fix

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:

    https://github.com/apache/flink/pull/1574#issuecomment-179138377
  
    Looks good to me, but @gumchum's comment is valid.
    
    One more thing, I would call the method on the Scala DataStream `def javaStream : JavaStream[T] = stream`. I think that is more in line with the Scala style: no parenthesis, because it is a simple accessor, and no "get" prefix, because for methods that appear like variables (no parenthesis), that seems the way they encourage it.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: Scala DataStream&DataStreamUtils accessors fix

Posted by cmonty <gi...@git.apache.org>.
Github user cmonty commented on the pull request:

    https://github.com/apache/flink/pull/1574#issuecomment-178683549
  
    The Scala interface looks good to me and gets rid of the need to call `getJavaStream` directly.
    
    Thanks!


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: Scala DataStream&DataStreamUtils accessors fix

Posted by mbalassi <gi...@git.apache.org>.
Github user mbalassi commented on the pull request:

    https://github.com/apache/flink/pull/1574#issuecomment-178823650
  
    Thanks for the comments. @gumchum: Internally it is stored as a Java `Iterator` and that is the reason why  left it so, but it definitely make sense to convert it to Scala before passing it to the users, so their code does not get "polluted" with Java stuff. I am modifying it.


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: Scala DataStream&DataStreamUtils accessors fix

Posted by gumchum <gi...@git.apache.org>.
Github user gumchum commented on the pull request:

    https://github.com/apache/flink/pull/1574#issuecomment-178783922
  
    Looks good. It looks like collect is returning a Java iterator instead of a Scala one. Is there a reason for this given that the code is for Scala users?


---
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 enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---