[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1471 Looks great to me. CI failure is unrelated (known flaky test https://issues.apache.org/jira/browse/ZEPPELIN-1623) Merge to master if no further comments. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 Those failures on travis seem unrelated - restarting build. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1471 @ravwojdyla Could you rebase to resolve conflict so we could merge 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1471 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1471 @ravwojdyla > Test/build should happen as part of standard build. Is it fine? Yes, sounds fantastic, thank you for pointing it out. Looks great to me, shall we merge it, if there is no further discussion? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 Rebased to solve conflict in `zeppelin-distribution/src/bin_license/LICENSE`. Are there any other comments? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @felixcheung correct - i missed it - updated doc/examples. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1471 @ravwojdyla reminder on my notes https://github.com/apache/zeppelin/pull/1471#issuecomment-255930386 ? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 ping @felixcheung --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1471 ah, sorry, I added a reply but it didn't go through apparently. I think that's good - I'm not sure if scio should be the default of the beam group (aren't there more Java users?) Could you update example, documentation and so on to reflect this change? I can do a pass after that --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 ping @felixcheung --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @felixcheung after going through the `BeamInterpreter`, I don't see immediate technical advantage of sharing the group, current implementation of `BeamInterpreter`, is actually pure Java interpreter, they would not share any configuration. That said from strictly logical point of view it makes sense for them to be in the same group - given they share the same model. What kind of changes are needed to put scio in the same group as beam? And what is gonna be the impact on 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1471 I think that's strictly cosmetic - you can see the Spark/R interpreter on how multiple interpreter classes are going to the same group. I think this helps discoverability and longer term transition when Scio becomes part of Beam. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @felixcheung I see, thanks for help, is 95dd9bb enough? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/1471 Hmm... perhaps you could advise us, would it be easy to switch over when it becomes part of beam? would it, say, make sense to put this into the beam group (`%beam.scio`) to help the transition and/or share the configuration now? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @felixcheung well - yes and no. scio is currently being contributed to beam, but current implementation still uses dataflow sdk. maybe we can consolidate it once scio migration to beam is done? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 ping @Leemoonsoo @felixcheung --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/1471 I don't have more comments. \cc @Leemoonsoo @felixcheung if they have comments. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @AhyoungRyu @zjffdu code was reviewed by @nevillelyh, build looks green. Any other comments? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @nevillelyh (the creator of scio) - could you please review. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user zjffdu commented on the issue: https://github.com/apache/zeppelin/pull/1471 @ravwojdyla I left some comments. And I am afraid no one in zeppelin community familiar with scio, could you ask someone else that know scio to review 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @zjffdu @AhyoungRyu are there any other comments? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 The tests are failing for: ``` Tests run: 9, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 119.221 sec <<< FAILURE! - in org.apache.zeppelin.integration.ParagraphActionsIT testWidth(org.apache.zeppelin.integration.ParagraphActionsIT) Time elapsed: 21.445 sec <<< FAILURE! java.lang.AssertionError: New Width is : 4 Expected: but: was at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20) at org.junit.Assert.assertThat(Assert.java:865) at org.junit.rules.ErrorCollector$1.call(ErrorCollector.java:65) at org.junit.rules.ErrorCollector.checkSucceeds(ErrorCollector.java:78) at org.junit.rules.ErrorCollector.checkThat(ErrorCollector.java:63) at org.apache.zeppelin.integration.ParagraphActionsIT.testWidth(ParagraphActionsIT.java:314) testWidth(org.apache.zeppelin.integration.ParagraphActionsIT) Time elapsed: 21.445 sec <<< FAILURE! java.lang.AssertionError: New Width is : 8 Expected: but: was at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20) at org.junit.Assert.assertThat(Assert.java:865) at org.junit.rules.ErrorCollector$1.call(ErrorCollector.java:65) at org.junit.rules.ErrorCollector.checkSucceeds(ErrorCollector.java:78) at org.junit.rules.ErrorCollector.checkThat(ErrorCollector.java:63) at org.apache.zeppelin.integration.ParagraphActionsIT.testWidth(ParagraphActionsIT.java:314) ``` I can't see the connection of those with this PR. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1471 @ravwojdyla Only the owner can re-trigger Travis job by closing & opening the PR :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @AhyoungRyu updated licenses. The travis failures do not seems to be related could you please restart the travis jobs? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @AhyoungRyu sorry I missed it. Will go ahead and add all the licenses. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1471 @ravwojdyla Here is the list of dependencies from `mvn dependency:tree`. All of them will become part of the Zeppelin binary package. What needed to be done is - make sure that all these dependencies and the transitive dependencies as well - have an appropriate licence, and are logged under the `./zeppelin-distribution/src/bin_license/LICENSE` - and for non-Apache license, full text of the license need to be added to `./zeppelin-distribution/src/license/` For your convenience I have listed all current dependencies of `scio` module below. You don't need to add all of the licenses listed in here. Many things are already existed. ``` [INFO] [INFO] Building Zeppelin: Scio 0.7.0-SNAPSHOT [INFO] [INFO] [INFO] --- maven-dependency-plugin:2.8:tree (default-cli) @ zeppelin-scio_2.10 --- [INFO] org.apache.zeppelin:zeppelin-scio_2.10:jar:0.7.0-SNAPSHOT [INFO] +- org.slf4j:slf4j-api:jar:1.7.10:compile [INFO] +- org.slf4j:slf4j-log4j12:jar:1.7.10:compile [INFO] | \- log4j:log4j:jar:1.2.17:compile [INFO] +- org.apache.zeppelin:zeppelin-interpreter:jar:0.7.0-SNAPSHOT:compile [INFO] | +- org.apache.thrift:libthrift:jar:0.9.2:compile [INFO] | | +- org.apache.httpcomponents:httpclient:jar:4.3.6:compile [INFO] | | | \- commons-codec:commons-codec:jar:1.5:compile [INFO] | | \- org.apache.httpcomponents:httpcore:jar:4.3.3:compile [INFO] | +- com.google.code.gson:gson:jar:2.2:compile [INFO] | +- org.apache.commons:commons-pool2:jar:2.3:compile [INFO] | \- org.apache.commons:commons-lang3:jar:3.4:compile [INFO] +- com.spotify:scio-repl_2.10:jar:0.2.3:compile [INFO] | +- com.spotify:scio-core_2.10:jar:0.2.3:compile [INFO] | | +- com.spotify:scio-bigquery_2.10:jar:0.2.3:compile [INFO] | | | +- com.google.apis:google-api-services-bigquery:jar:v2-rev317-1.22.0:compile [INFO] | | | \- org.joda:joda-convert:jar:1.8.1:compile [INFO] | | +- com.google.cloud.dataflow:google-cloud-dataflow-java-sdk-all:jar:1.7.0:compile [INFO] | | | +- com.google.apis:google-api-services-dataflow:jar:v1b3-rev36-1.22.0:compile [INFO] | | | +- io.grpc:grpc-all:jar:0.13.1:compile [INFO] | | | | +- io.grpc:grpc-auth:jar:0.13.1:compile [INFO] | | | | +- io.grpc:grpc-netty:jar:0.13.1:compile [INFO] | | | | | \- io.netty:netty-codec-http2:jar:4.1.0.CR1:compile [INFO] | | | | | \- io.netty:netty-codec-http:jar:4.1.0.CR1:compile [INFO] | | | | +- io.grpc:grpc-protobuf:jar:0.13.1:compile [INFO] | | | | +- io.grpc:grpc-core:jar:0.13.1:compile [INFO] | | | | +- io.grpc:grpc-okhttp:jar:0.13.1:compile [INFO] | | | | | +- com.squareup.okio:okio:jar:1.6.0:compile [INFO] | | | | | \- com.squareup.okhttp:okhttp:jar:2.5.0:compile [INFO] | | | | +- io.grpc:grpc-protobuf-nano:jar:0.13.1:compile [INFO] | | | | | \- com.google.protobuf.nano:protobuf-javanano:jar:3.0.0-alpha-5:compile [INFO] | | | | \- io.grpc:grpc-stub:jar:0.13.1:compile [INFO] | | | +- io.netty:netty-handler:jar:4.1.0.CR1:compile [INFO] | | | | +- io.netty:netty-buffer:jar:4.1.0.CR1:compile [INFO] | | | | | \- io.netty:netty-common:jar:4.1.0.CR1:compile [INFO] | | | | +- io.netty:netty-transport:jar:4.1.0.CR1:compile [INFO] | | | | | \- io.netty:netty-resolver:jar:4.1.0.CR1:compile [INFO] | | | | \- io.netty:netty-codec:jar:4.1.0.CR1:compile [INFO] | | | +- com.google.api.grpc:grpc-pubsub-v1:jar:0.0.2:compile [INFO] | | | | \- com.google.api.grpc:grpc-core-proto:jar:0.0.3:compile [INFO] | | | +- com.google.auth:google-auth-library-oauth2-http:jar:0.4.0:compile [INFO] | | | | \- com.google.auth:google-auth-library-credentials:jar:0.4.0:compile [INFO] | | | +- com.google.cloud.bigtable:bigtable-protos:jar:0.3.0:compile [INFO] | | | +- com.google.api-client:google-api-client:jar:1.22.0:compile [INFO] | | | +- com.google.api-client:google-api-client-java6:jar:1.22.0:compile [INFO] | | | +- com.google.api-client:google-api-client-jackson2:jar:1.22.0:compile [INFO] | | | +- com.google.apis:google-api-services-clouddebugger:jar:v2-rev8-1.22.0:compile [INFO] | | | +- com.google.apis:google-api-services-pubsub:jar:v1-rev10-1.22.0:compile [INFO] | | | +- com.google.apis:google-api-services-storage:jar:v1-rev71-1.22.0:compile [INFO] | | | +- com.google.http-client:google-http-client:jar:1.22.0:compile [INFO] | | | +- com.google.http-client:google-http-client-jackson:jar:1.22.0:compile [INFO] | | | +-
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1471 @ravwojdyla Can you add `docs/interpreter/scio.md` path below [this line](https://github.com/apache/zeppelin/blob/master/docs/_includes/themes/zeppelin/_navigation.html#L68)? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 ping @AhyoungRyu @zjffdu --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @AhyoungRyu @zjffdu are there any more comment, how do you want to proceed? the error on travis does not seems to be related? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes [ZEPPELIN-1505] Add Scio interpreter
Github user ravwojdyla commented on the issue: https://github.com/apache/zeppelin/pull/1471 @AhyoungRyu @zjffdu could you please review when your have time. all the todos are âï¸ --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1471: Closes ZEPPELIN-1505: Add Scio interpreter
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1471 @ravwojdyla Thanks for your contribution and welcome! I assume this PR is not finished but `Work In Process` since there are some unchecked TODOs. So how about changing the title of this PR from `Closes ZEPPELIN-1505: Add Scio interpreter ` to `[WIP][ZEPPELIN-1505] Add Scio interpreter` or sth? It would be more helpful to anyone who want to understand the purpose of this PR at a glance and start review :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
