Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

talpr-50-flow-tagger #52

Closed
wants to merge 4 commits into from
Closed

Conversation

talpr
Copy link

@talpr talpr commented Aug 26, 2016

FlowTagger (issue #50)

@talpr talpr mentioned this pull request Aug 27, 2016
Fix FocusedFlowOps.focus types
Test FocusedFlow
Remove unnecessary @uncheckedVariance
setHandler(inTup, new InHandler {
override def onPush(): Unit = {
val (cur, in) = grab(inTup)
curr = cur
Copy link
Member

@ktoso ktoso Aug 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make it passed in which value we pick :-)
This would be doable via magnet actually hm hm.

Copy link
Member

@ktoso ktoso Aug 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually going with raw, sbt-boilerplate generated ones would be enough it seems,
and it also seems we do need a custom type, SubFlow won't do hm.

General idea would be:

  // need to generate for 22-arity
  implicit final class Focus2OnSource[+T1, +T2, +M](s: Source[(T1, T2), M]) {
  import TupleGet._
    def focus[T, TT1, TT2]( // fix variance
      get: Tuple2[T1, T2] => T, 
      set: Tuple2[T1, T2] => T => Tuple2[TT1, TT2]): impl.SubFlowImpl[T, T, M, scaladsl.Source[Out, M]#Repr, scaladsl.RunnableGraph[M]] = ???
  }

  Source.single("" -> 12)
    .focus(get = t => t._1, 
           set = t => v => t.copy(_1 = v))

And instead of the SubFlow we'd need a custom type there it seems though – so yeah FocusedSource indeed so we have the type to unfocus.

Would you be able to give it a shot towards n-arity? Then it would be really awesome and useful I think

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktoso
So this brings up a couple of related questions:

  • If the user has to provide the 'get' and 'set' mappings, then why limit it to tuples?
  • If it is limited to tuples, then wouldn't it be better to generate methods like focus_1, focus_2,... ?
  • What happens if the output of the flow is a different type from the input?

Originally I thought about having the user to provide the functions when calling focus/unfocus, but that made the API really cluttered and I don't think it saved too much (considering that the user can just add a 'map' themselves).

So what I'm trying to say is, I'll gladly try to make it work for larger tuples, but I'm not sure whether it really adds much.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why I mentioned Lenses. They are basically reified getters/setters for immutable data. While it probably needs some good lense library (I think shapeless have them) they could work nicely. It is Scala only of course.

@ktoso
Copy link
Member

ktoso commented Aug 29, 2016

Very fun, hope to see the full thing soon :)

@drewhk
Copy link
Member

drewhk commented Aug 29, 2016

On the side: one fun idea in a similar vein would be to accept Lenses to "project-down" a stream. Then you would be able to handle any n-to-n streams, filtering, mapConcatting or whatever on types, given that there is a Lens for it.

eg. src.project(userField).filter(_ == "johndoe").unproject

def focus = new FocusedFlow(BidiFlow.fromFlowsMat(from, Flow[In])(Keep.left))
}

def main(args: Array[String]): Unit = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about this main method? We would need unit tests instead :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, this is still a draft... ^_^

@drewhk
Copy link
Member

drewhk commented Aug 29, 2016

Hm, this approach seems to be dangerous to me. It works for filter, but I don't see how it can work for 1-to-n stages. Also, isn't this racy? I mean the unfocus part and the tag it applies.

@talpr
Copy link
Author

talpr commented Aug 29, 2016

@drewhk
Indeed, it won't always work. 1-to-n is OK (assuming you expect them all to get the same tag), but anything that has internal buffers or anything happening out-of-order won't work.

Like I said in the original issue, this won't always work, but in many simple use-cases it can be very handy.

@drewhk
Copy link
Member

drewhk commented Aug 29, 2016

I just investigate if we can make it work better. For example if not only via is overridden, but mapAsync to automatically remove then add the tag, it can work, too.

@talpr
Copy link
Author

talpr commented Aug 29, 2016

Yeah, that could work. It would probably require unfocusing the stream first, doing the mapAsync, and focusing again, but I guess that wouldn't be that bad...

@ennru
Copy link
Member

ennru commented Mar 22, 2019

This hasn't seen any activity for a very long time.
I'll close it, please re-open or start over.

@ennru ennru closed this Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants