-
Notifications
You must be signed in to change notification settings - Fork 267
Conversation
|
||
JProducer<P, T> filter(Function1<T, Boolean> f); | ||
|
||
// TODO java does not allow U super T here. used T |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you are not doing variance in this API, and you are keeping JProducer invariant, I think it is fine to just T.
here are some examples with different versions of java: |
})).sumByKey(store, sg); // store is typed Object | ||
} | ||
|
||
public static <P extends Platform<P>> void wordCount2(JProducer<P, Status> source, Store<P, ?, String, Long> store) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking of removing that 2nd type parameter in Store, Service, Sink, ...
This is the actual type used by the platform, but the Producer API does not care at that point because those types have been erased by the scala compiler and replaced by Object.
The downside of that parameter is the summingbird job code must use "?" to say that is is not using it. The upside is that otherwise the Wrapper class would just contain Object when possibly we may want to access the actual type to interact with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also allows people to write ill-formed jobs, and that will happen frequently. The confusion around why online stores are different from offline stores will be amplified.
This kind of type parameter is called a phantom type, and it is there to ensure that everything is consistent. Without this, you will just have to fail at runtime.
I know java users are used to that, but our goal in summingbird (scala) is that if it compiles, it runs.
the build for scala 2.9 fails because Either has moved from the scala package to the scala.util package between 2.9 and 2.10. |
} | ||
} | ||
|
||
public static <T> Semigroup<T> semigroup(Class<T> c) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add issue on algebird linking to this that we can move to algebird-java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and pull request:
twitter/algebird#317
We should probably move this code into a summingbird-java target? |
|
||
import com.twitter.summingbird.Platform; | ||
|
||
public class Buffer<P extends Platform<P>, BUFFER, K, V> extends Wrapper<BUFFER> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does K and V come into play?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are the type parameters defined in Platform#Buffer[K,V]
summingbird/summingbird-core/src/main/scala/com/twitter/summingbird/Platform.scala
Line 32 in 8d16fad
type Buffer[k, v] = Service[k, v] with Sink[(k, v)] |
K is the key we do the join (or lookup) on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but I mean, we never use them on our side of the code, right? There's no connection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is checked here:
https://github.com/julienledem/summingbird/blob/d88553b193c24b39175bb19e3bab46deb5006997/summingbird-core/src/main/java/com/twitter/summingbird/javaapi/JKeyedProducer.java#L28
The platform will provide the right types. Those parameters are here to be carried to the Producer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can not express that in java but BUFFER is SOMETHING<K,V>
and the Platform provides the right type.
See for example with Store:
https://github.com/julienledem/summingbird/blob/d88553b193c24b39175bb19e3bab46deb5006997/summingbird-core/src/main/java/com/twitter/summingbird/memory/javaapi/JMemory.java#L38
return (Option<T>)scala.None$.MODULE$; | ||
} | ||
|
||
public static <T> Option<T> some(T t) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if they pass null? This may trip up. In scala this shouldn't be possible but from Java it may be. If you call Option.apply, a null will become a None. Or you can throw an exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix this.
Except for some nits, looks reasonable. Let's make a big fat EXTREMELY EXPERIMENTAL on this and try to get people banging on it. Also: should be in a separate package, per our discussions. Make sure that when you do that you add it to the aggregate so the tests are run. |
…ample Conflicts: project/Build.scala
Experimental Java API and example
Thanks Julien! And god help anyone who wants to use it ;) |
A Java API to make summingbird easier to use from java (including java 8).