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
Utility methods for running Executions in parallel #1507
Conversation
Execution.from(lock.acquire()) | ||
.flatMap(_ => e) | ||
.onComplete { | ||
case Success(_) => lock.release() |
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.
Should we not be releasing on success or failure?
Why not use scala futures? |
@isnotinvain I guess you mean Twitter futures. Answer: the deps are already heavy adding util just to get this method is not worth it. Mo deps mo problems. |
Let me know what you guys think, pulled in AsyncSemaphore and made it work with scala futures. Made it private also since I have really only tested it for this use case. I also hopefully came up with a test that stresses the parallelism. |
case Success((_, p)) => p.release() | ||
case Failure(_) => () | ||
} | ||
.map(_._1.get) |
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.
rather than map(... get)
can we call flatMap(... fromTry( )...)
Utility methods for running Executions in parallel
Pulling this over from the Twitter world. I decided against pulling it the twitter promise library here and instead use the java concurrency library for the locking. Looking at the documents the methods look thread safe.
I am also open to a better name for the for the class. I was going to them on the Execution object but that didn't seem correct.