Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-109875609
Great news :) Thanks Ufuk!
---
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 t
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-109868329
Initial version is merged in DISABLE mode! Thanks for the contribution. I
will file some follow up JIRAs. :)
---
If your project is set up for it, you can reply to this emai
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/729
---
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
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-109290186
Travis is running... https://github.com/uce/incubator-flink/tree/sca :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-109264887
Good points. Perfectly fine for me ;)
---
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
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-109259623
Thanks for the changes. :-) They look very good. Cluster tests are also
fine.
---
I would like to address the following points, squash your commits and merge
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31799272
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/functions/SemanticPropUtil.java
---
@@ -309,15 +308,20 @@ public static DualInputSemanticProperti
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31798470
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/functions/SemanticPropUtil.java
---
@@ -309,15 +308,20 @@ public static DualInputSemanticProperti
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31711766
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/functions/SemanticPropUtil.java
---
@@ -309,15 +308,20 @@ public static DualInputSemanticProperti
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31707016
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/SingleInputUdfOperator.java
---
@@ -54,8 +54,11 @@
private Map> broadcast
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-108808990
The build succeeds. :) I will have a look at the changes. Thanks for not
force updating this PR.
I will test it in a distributed setup and if everything runs fine, we
Github user fhueske commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31706246
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/SingleInputUdfOperator.java
---
@@ -54,8 +54,11 @@
private Map> broadcast
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-108640926
Hey Ufuk,
thank you very much for reviewing my code and all others for the feedback!
I tried to consider all your feedback (I hope I didn't forget anything). I di
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-108630545
Great review, Ufuk.
I agree with @uce and @rmetzger to add a comment how to disable it (in
case).
---
If your project is set up for it, you can reply to thi
Github user twalthr commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31597259
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/SingleInputUdfOperator.java
---
@@ -54,8 +54,11 @@
private Map> broadcast
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-108093699
I agree with Robert, but for the initial version it won't matter as it
should be disabled anyways.
---
If your project is set up for it, you can reply to this email and have
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-108081964
How about extending the `UDF contains obvious errors` message with some
notes on how to completely disable the SCA.
I fear that the message appears (blocks the progra
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-107996871
Hey Timo,
I've tested this locally so far and it's working smoothly! Let's write a
blog post about this very soon. :-) As soon as I have access to more machines,
I w
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31535397
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/UdfOperatorUtils.java
---
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31532974
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/UdfOperatorUtils.java
---
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31532320
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/sca/UdfAnalyzer.java ---
@@ -0,0 +1,431 @@
+/*
+ * Licensed to the Apache Software Foundation
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31531993
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/SingleInputUdfOperator.java
---
@@ -54,8 +54,11 @@
private Map> broadcastVari
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31531758
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/UdfOperatorUtils.java
---
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31530742
--- Diff:
flink-java/src/main/java/org/apache/flink/api/java/operators/SingleInputUdfOperator.java
---
@@ -54,8 +54,11 @@
private Map> broadcastVari
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31528084
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/UdfAnalysisMode.java ---
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31527970
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/UdfAnalysisMode.java ---
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/729#discussion_r31527840
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/UdfAnalysisMode.java ---
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-107941730
> Naming UDF. The feedback of committers giving talks about Flink some time
ago was that the name "UDF" was sometimes confusing. @rmetzger, can you confirm
this? We migh
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-107924813
I understand that concern.
But using sysoutput will also be interleaved with the Client sysout
printing and the regular system logging.
Also, I think its ver
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-107482998
The concern about logging is that, when using the local mode inside the
IDE, the system logs a lot and the hints get lost.
If you don't want sysoutput, you co
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-107422340
+1 for the annotation.
I'm against using stdout. Logging frameworks are much better at controlling
the output.
The quickstart mvn archetype provides a log4j.
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-107419997
OK, I will review this.
I vote to stick to Stephan's suggested approach instead of package based
exclusions: analyze everything and allow exclusions with a `@SkipCode
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-107393190
Let us merge this for 0.9 and have it deactivated by default.
Let's gradually activate it in the next releases as it gets exposure
---
If your project is set
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-106243626
The `collect()` is a special case, the analyzer does not follow it. But
after thinking about it I recognized that I forgot `getRuntimeContext()` I will
fix this ;)
---
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-106241536
- I agree with hints going to sysout and activating by default.
- For simple functions, most of the transitive allocations will be Flink
internal, right? E.g. after callin
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-106236797
+1 for activating hinting locally.
I thought logging is the standard way to print, but I can change it to a
sysout
---
If your project is set up for it, you can rep
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-106233384
I disabled the analyzer for all classes starting with "org.apache.flink".
Because I wanted to reduce the output to the user for build-in UDFs (e.g.
`org.apache.flink.api.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-106232545
I second Ufuk's comments.
Merging it and deactivating it by default. I can see a 0.9.1 or 0.10.0
release coming in very soon afterwards, because we have a big set
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-106093530
This is a very cool! Thanks for all the effort you put into this. I think
this will be a great addition to the project. :-)
---
I've skimmed over the code an
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-105926378
Okay, pending a proper distributed test, you have my vote to add this!
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user twalthr commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-105897738
The bytecode generated from Scala Compiler is the same. That is not a
problem for the analyzer. But Scala is not fully supported yet, because of the
different Java/Scala
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-105840358
The tests are failing with checkstyle violations. Can you fix those?
---
If your project is set up for it, you can reply to this email and have your
reply appear on G
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-105816818
Looks very nice, seems to have a good test coverage as well.
How well does it work with bytecode generated by the Scala Compiler?
---
If your project is set
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-105814322
Indeed, this looks like an impressive addition.
Let's get it into 0.9 as a beta feature!
---
If your project is set up for it, you can reply to this email an
Github user rmetzger commented on the pull request:
https://github.com/apache/flink/pull/729#issuecomment-105799678
Thanks a lot for this great work!
I'll soon try out the code to see how it works.
Given that we can disable it and that its not automatically setting
semanti
GitHub user twalthr opened a pull request:
https://github.com/apache/flink/pull/729
[FLINK-1319][core] Add static code analysis for UDFs
This PR implements a Static Code Analyzer (SCA) that uses the ASM framework
for interpreting Java bytecode of Flink UDFs. The analyzer is build on
46 matches
Mail list logo
