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

Support Streaming for Armeria backend #860

Merged
merged 8 commits into from Feb 22, 2021

Conversation

ikhoon
Copy link
Contributor

@ikhoon ikhoon commented Feb 14, 2021

Motivation:

Armeria natively supports Streaming through Reactive Streams

Modifications:

  • Introduce AbstractArmeriaBackend for common conversions
  • Support streaming using Publisher of Reactive Streams
  • Add new modules for better interop with other libraries
    • FS2
    • Monix
    • Cats
    • Scalaz
    • ZIO

Result:

Better interop Armeria with sttp.

Before submitting pull request:

  • Check if the project compiles by running sbt compile
  • Verify docs compilation by running sbt compileDocs
  • Check if tests pass by running sbt test
  • Format code by running sbt scalafmt

Motivation:

Armeria natively supports Streaming through Reactive Streams

Modifications:

- Introduce `AbstractArmeriaBackend` for common conversions
- Support streaming using `Publisher` of Reactive Streams
- Add new modules for better interops with other libraries
  - FS2
  - Monix
  - Cats
  - Scalaz
  - ZIO

Result:

Better interop Armeria with sttp.
class ArmeriaZioHttpTest extends HttpTest[Task] with ZioTestBase {

// FIXME(ikhoon): A request failed with `ResponseTimeoutException`.
// However, "read exceptions - timeout" test never ends.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why read exceptions - timeout test never ends.
Can I get some advice?

Copy link
Member

Choose a reason for hiding this comment

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

does this happen only for the zio backend? and both locally and on CI?

Copy link
Contributor Author

@ikhoon ikhoon Feb 16, 2021

Choose a reason for hiding this comment

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

  • It only happens for the ZIO backend.
  • It is reproducible locally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a bug in closing a client factory. The problem is fixed now.

@ikhoon
Copy link
Contributor Author

ikhoon commented Feb 14, 2021

/cc @ngbinh

@ngbinh
Copy link

ngbinh commented Feb 14, 2021

Awesome. 🤩

@ikhoon
Copy link
Contributor Author

ikhoon commented Feb 20, 2021

All known failed tests are fixed. Not sure that the failed tests in CI are related to this PR.
sbt test passed locally.

@adamw adamw merged commit 765cd2c into softwaremill:master Feb 22, 2021
@ikhoon ikhoon deleted the armeria-streaming branch February 22, 2021 11:57
@ikhoon
Copy link
Contributor Author

ikhoon commented Feb 22, 2021

@adamw Thanks for the quick merge. I left a comment on your revise.
d9e6589#commitcomment-47411687

@ngbinh
Copy link

ngbinh commented Feb 23, 2021

awesome! Will try armeria backend when the new sttp release.

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.

None yet

3 participants