http: deprecate more of coding infrastructure #3294

jrudolph · 2020-06-24T14:42:50Z

Previously, we exposed Scala objects as API directly which exposes implementation
details and makes it hard to evolve the implementation. Instances in coders
are upcasted to Coder so that implementation details are not leaked any more.

…zip etc Refs akka#2916 Previously, we exposed Scala objects as API directly which exposes implementation details and makes it hard to evolve the implementation. Instances in coders are upcasted to `Coder` so that implementation details are not leaked any more.

akka-ci · 2020-06-24T14:46:45Z

Test FAILed.

!!! Couldn't read commit file !!!

akka-ci · 2020-06-24T14:50:48Z

Test FAILed.

!!! Couldn't read commit file !!!

ennru

LGTM.

ennru · 2020-06-24T16:23:39Z

The doc examples show Gzip.withLevel(9) which I silenced for now.

akka-ci · 2020-06-24T16:37:30Z

Test FAILed.

Pull request validation report

Failed Test Suites

Test result for 'akka-http-tests / Pr-validation / ./ executeTests'

[info] ScalaTest
[info] Run completed in 3 minutes, 21 seconds.
[info] Total number of tests run: 1243
[info] Suites: completed 63, aborted 0
[info] Tests: succeeded 1237, failed 6, canceled 0, ignored 0, pending 39
[info] *** 6 TESTS FAILED ***
[error] Failed: Total 1400, Failed 8, Errors 0, Passed 1392, Pending 39
[error] Failed tests:
[error] 	akka.http.scaladsl.server.directives.CodingDirectivesSpec
[error] 	akka.http.javadsl.server.directives.CodingDirectivesTest
[error] 	akka.http.scaladsl.coding.GzipSpec

Test result for 'docs / Pr-validation / ./ executeTests'

[info] ScalaTest
[info] Run completed in 3 minutes, 20 seconds.
[info] Total number of tests run: 291
[info] Suites: completed 60, aborted 0
[info] Tests: succeeded 289, failed 2, canceled 0, ignored 0, pending 0
[info] *** 2 TESTS FAILED ***
[error] Failed: Total 511, Failed 4, Errors 0, Passed 494, Skipped 13
[error] Failed tests:
[error] 	docs.http.javadsl.server.directives.CodingDirectivesExamplesTest
[error] 	docs.http.scaladsl.server.directives.CodingDirectivesExamplesSpec

jrudolph · 2020-06-24T14:45:37Z

akka-http/src/main/scala/akka/http/scaladsl/coding/Coders.scala

+import scala.collection.immutable
+
+@silent("in package coding is deprecated")
+object Coders {


This structure follows the javadsl more closely. However, on the java side, they part of enum Coder (singular term). Should we move those into the Coder companion object as well? In other parts of the (model) API, we tried to put those instances into a plural-named object, so that's why I went with that.

Good to be consistent with e.g. StatusCodes I think, indeed

jrudolph · 2020-06-24T14:46:43Z

akka-http/src/main/scala/akka/http/scaladsl/coding/Encoder.scala

  def encode(input: ByteString): ByteString = newCompressor.compressAndFinish(input)

+  def encodeAsync(input: ByteString)(implicit mat: Materializer): Future[ByteString] =


Unfortunately, the non-async version is already taken, so it's slightly inconsistent between decoding and encoding side.

jrudolph · 2020-06-25T07:44:07Z

The doc examples show Gzip.withLevel(9) which I silenced for now.

Thanks, I tried to go through the warnings, but it seems adding a deprecation didn't recompile dependencies so I missed all of those.

akka-ci · 2020-06-25T08:28:43Z

Test FAILed.

Pull request validation report

Failed Test Suites

Test result for 'akka-http-tests / Pr-validation / ./ executeTests'

[info] ScalaTest
[info] Run completed in 3 minutes, 23 seconds.
[info] Total number of tests run: 1243
[info] Suites: completed 63, aborted 0
[info] Tests: succeeded 1237, failed 6, canceled 0, ignored 0, pending 39
[info] *** 6 TESTS FAILED ***
[error] Failed: Total 1400, Failed 8, Errors 0, Passed 1392, Pending 39
[error] Failed tests:
[error] 	akka.http.scaladsl.server.directives.CodingDirectivesSpec
[error] 	akka.http.javadsl.server.directives.CodingDirectivesTest
[error] 	akka.http.scaladsl.coding.GzipSpec

Test result for 'docs / Pr-validation / ./ executeTests'

[info] ScalaTest
[info] Run completed in 3 minutes, 22 seconds.
[info] Total number of tests run: 291
[info] Suites: completed 60, aborted 0
[info] Tests: succeeded 289, failed 2, canceled 0, ignored 0, pending 0
[info] *** 2 TESTS FAILED ***
[error] Failed: Total 511, Failed 4, Errors 0, Passed 494, Skipped 13
[error] Failed tests:
[error] 	docs.http.javadsl.server.directives.CodingDirectivesExamplesTest
[error] 	docs.http.scaladsl.server.directives.CodingDirectivesExamplesSpec

It seems `encodeAsync` which is now used for comparison is slightly less efficient than the previous sync version

jrudolph · 2020-06-25T10:10:55Z

akka-http/src/main/scala/akka/http/scaladsl/coding/Coders.scala

+  def Gzip: Coder = akka.http.scaladsl.coding.Gzip
+  def Gzip(messageFilter: HttpMessage => Boolean): Coder = new Gzip(messageFilter)
+
+  def Deflate: Coder = akka.http.scaladsl.coding.Gzip


akka-ci · 2020-06-25T10:14:27Z

Test FAILed.

!!! Couldn't read commit file !!!

akka-ci · 2020-06-25T10:25:32Z

Test PASSed.

raboof · 2020-06-25T11:09:19Z

akka-http/src/main/scala/akka/http/scaladsl/coding/Coders.scala

+import scala.collection.immutable
+
+@silent("in package coding is deprecated")
+object Coders {


Good to be consistent with e.g. StatusCodes I think, indeed

raboof · 2020-06-25T11:10:34Z

akka-http/src/main/scala/akka/http/scaladsl/coding/Gzip.scala

 import akka.http.scaladsl.model._
 import akka.http.scaladsl.model.headers.HttpEncodings

-class Gzip private (compressionLevel: Int, val messageFilter: HttpMessage => Boolean) extends Coder with StreamDecoder {
+@InternalApi
+@deprecated("Actual implementation of Gzip is internal, use Coders.Gzip instead", since = "10.2.0")


It's a bit strange to mark internal API as deprecated, but indeed as it used to be public API I guess it makes sense 👍

Yeah, guess I went over the top with deprecating stuff, just to be sure. Maybe we can create an internal copy later as the future version while keeping that one around only for compatibility.

ennru · 2020-06-25T11:20:10Z

I did another round on this to use the non-deprecated variants in the tests.

akka-ci · 2020-06-25T11:26:33Z

Test PASSed.

jrudolph · 2020-06-25T11:28:48Z

I did another round on this to use the non-deprecated variants in the tests.

Thanks, much appreciated :)

I'm planning to do a more thorough review of warnings and documentation for 10.2.0 next week.

jrudolph requested a review from raboof June 24, 2020 14:42

jrudolph force-pushed the 2916-deprecate-more-compression-infra branch from 269cf49 to 90dd6a4 Compare June 24, 2020 14:43

akka-ci added the validating PR that is currently being validated by Jenkins label Jun 24, 2020

jrudolph force-pushed the 2916-deprecate-more-compression-infra branch from 90dd6a4 to 2718dee Compare June 24, 2020 14:44

Autoformatted code

a1477b4

jrudolph mentioned this pull request Jun 24, 2020

Remove Deflate/GZip stream stages and use ones from akka-stream #2916

Open

ennru approved these changes Jun 24, 2020

View reviewed changes

Update doc examples

eca35b4

akka-ci added validating PR that is currently being validated by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) labels Jun 24, 2020

akka-ci added needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR that is currently being validated by Jenkins labels Jun 24, 2020

jrudolph commented Jun 25, 2020

View reviewed changes

Merge branch 'master' into 2916-deprecate-more-compression-infra

58c3ffe

akka-ci removed the validating PR that is currently being validated by Jenkins label Jun 25, 2020

jrudolph added 3 commits June 25, 2020 12:09

disable Gzip compression rate test

34e9d23

It seems `encodeAsync` which is now used for comparison is slightly less efficient than the previous sync version

replace pasta with meat

c67684f

deprecate withLevel in favor of new constructors in Coders

9f34b08

jrudolph commented Jun 25, 2020

View reviewed changes

fix examples

ee85d43

akka-ci added validating PR that is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR that is currently being validated by Jenkins labels Jun 25, 2020

raboof approved these changes Jun 25, 2020

View reviewed changes

Use less deprectated things

6cc3652

akka-ci added validating PR that is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed tested PR that was successfully built and tested by Jenkins validating PR that is currently being validated by Jenkins labels Jun 25, 2020

jrudolph changed the title ~~http: deprecate more of coding infrastructure, coders now in Coders.Gzip etc~~ http: deprecate more of coding infrastructure Jun 25, 2020

jrudolph merged commit bd818e2 into akka:master Jun 25, 2020

jrudolph deleted the 2916-deprecate-more-compression-infra branch June 25, 2020 11:31

jrudolph added this to the 10.2.0 milestone Jun 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

http: deprecate more of coding infrastructure #3294

http: deprecate more of coding infrastructure #3294

jrudolph commented Jun 24, 2020 •

edited

Loading

akka-ci commented Jun 24, 2020

akka-ci commented Jun 24, 2020

ennru left a comment

ennru commented Jun 24, 2020

akka-ci commented Jun 24, 2020

jrudolph Jun 24, 2020

raboof Jun 25, 2020

jrudolph Jun 24, 2020

jrudolph commented Jun 25, 2020

akka-ci commented Jun 25, 2020

jrudolph Jun 25, 2020

akka-ci commented Jun 25, 2020

akka-ci commented Jun 25, 2020

raboof Jun 25, 2020

raboof Jun 25, 2020

jrudolph Jun 25, 2020

ennru commented Jun 25, 2020

akka-ci commented Jun 25, 2020

jrudolph commented Jun 25, 2020

		def encode(input: ByteString): ByteString = newCompressor.compressAndFinish(input)

		def encodeAsync(input: ByteString)(implicit mat: Materializer): Future[ByteString] =

http: deprecate more of coding infrastructure #3294

http: deprecate more of coding infrastructure #3294

Conversation

jrudolph commented Jun 24, 2020 • edited Loading

akka-ci commented Jun 24, 2020

akka-ci commented Jun 24, 2020

ennru left a comment

Choose a reason for hiding this comment

ennru commented Jun 24, 2020

akka-ci commented Jun 24, 2020

Pull request validation report

jrudolph Jun 24, 2020

Choose a reason for hiding this comment

raboof Jun 25, 2020

Choose a reason for hiding this comment

jrudolph Jun 24, 2020

Choose a reason for hiding this comment

jrudolph commented Jun 25, 2020

akka-ci commented Jun 25, 2020

Pull request validation report

jrudolph Jun 25, 2020

Choose a reason for hiding this comment

akka-ci commented Jun 25, 2020

akka-ci commented Jun 25, 2020

raboof Jun 25, 2020

Choose a reason for hiding this comment

raboof Jun 25, 2020

Choose a reason for hiding this comment

jrudolph Jun 25, 2020

Choose a reason for hiding this comment

ennru commented Jun 25, 2020

akka-ci commented Jun 25, 2020

jrudolph commented Jun 25, 2020

jrudolph commented Jun 24, 2020 •

edited

Loading