Skip to content

Conversation

@olafurpg
Copy link
Contributor

@olafurpg olafurpg commented Jun 26, 2020

Problem

Currently, the primary syntax to compose Twitter futures is using map/flatMap syntax. This syntax may feel verbose and unfamiliar to developers who have worked with other languages such as JavaScript or C# that support async/await.

Solution

This PR adds async/await support for com.twitter.util.Future. This functionality is
only available in the newly released 2.13.3 (notes https://github.com/scala/scala/releases/tag/v2.13.3). It will also become available in the upcoming 2.12.12 release.

Result

Users can write code like this now

val c = async {
  val a = Future.value(10)
  val b = Future.value(5)
  await(a) + await(b)
}
assert(c.poll == Some(Return(15)))

The equivalent way to to write the same code above with the current syntax is

  val c = for {
    a <- Future.value(10)
    b <- Future.value(5)
  } yield a + b

Notes

The code in this PR is adapted from the code in scala/scala-async#237

To try out this PR locally run the following commands

$ sbt
> ++2.13.3
> util-core/testOnly com.twitter.util.AsyncTest

@olafurpg
Copy link
Contributor Author

For the record, I'm not very familiar with Twitter futures or the new -Xasync flag. This PR is just an experiment so I'm converting it into a draft.

@olafurpg olafurpg marked this pull request as draft June 26, 2020 11:40
This commit adds support to compose Twitter futures using async/await
style syntax instead of map/flatMap syntax. This functionality is
only available in the newly released 2.13.3 (notes
    https://github.com/scala/scala/releases/tag/v2.13.3) and it will
also become available in the upcoming 2.12.12 release.
@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2020

Codecov Report

Merging #279 into develop will increase coverage by 0.62%.
The diff coverage is 64.28%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #279      +/-   ##
===========================================
+ Coverage    46.12%   46.75%   +0.62%     
===========================================
  Files          238      238              
  Lines        14414    14063     -351     
  Branches       869      886      +17     
===========================================
- Hits          6649     6575      -74     
+ Misses        7765     7488     -277     
Impacted Files Coverage Δ
...l-core/src/main/scala/com/twitter/util/Async.scala 0.00% <0.00%> (ø)
...in/scala/com/twitter/util/FutureStateMachine.scala 69.23% <69.23%> (ø)
.../src/main/scala/com/twitter/util/ThriftCodec.scala 80.00% <0.00%> (-5.72%) ⬇️
.../main/scala/com/twitter/util/logging/Logging.scala 26.31% <0.00%> (-1.89%) ⬇️
...twitter/finagle/stats/BroadcastStatsReceiver.scala 49.05% <0.00%> (-1.86%) ⬇️
...r/finagle/stats/NameTranslatingStatsReceiver.scala 87.50% <0.00%> (-1.39%) ⬇️
...l-zk/src/main/scala/com/twitter/zk/Connector.scala 61.29% <0.00%> (-1.21%) ⬇️
...c/main/scala/com/twitter/util/routing/Router.scala 88.88% <0.00%> (-1.12%) ⬇️
...c/main/scala/com/twitter/logging/FileHandler.scala 82.97% <0.00%> (-0.86%) ⬇️
...ore/src/main/scala/com/twitter/util/Duration.scala 84.97% <0.00%> (-0.58%) ⬇️
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c074a5f...8130281. Read the comment docs.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Olafur Pall Geirsson seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@olafurpg olafurpg closed this Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants