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

core: polishing and docs for ServerBuilder API #3382

Merged
merged 8 commits into from
Jul 21, 2020

Conversation

jrudolph
Copy link
Member

  • One significant change over http: new ServerBuilder API #3362: I renamed bind() to connectionSource to make it clear that calling the method will not immediately create a binding and also to avoid overloading bind() (which became awkward in docs)
  • Converted remaining internal and docs usages of Http().bindXYZ usages
  • Brief release notes and migration notes entry

This should signify that the binding is not yet done when the method is
called but only later when the source is materialized.
@jrudolph jrudolph added this to the 10.2.0 milestone Jul 21, 2020
@jrudolph jrudolph requested a review from raboof July 21, 2020 10:05
@akka-ci akka-ci added validating PR that is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR that is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) labels Jul 21, 2020
@akka-ci akka-ci added needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR that is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed validating PR that is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) labels Jul 21, 2020
@akka-ci
Copy link

akka-ci commented Jul 21, 2020

Test PASSed.

akka-http-core/src/main/resources/reference.conf Outdated Show resolved Hide resolved
*/
def bind(): Source[IncomingConnection, CompletionStage[ServerBinding]]
def connectionSource(): Source[IncomingConnection, CompletionStage[ServerBinding]]
Copy link
Member

Choose a reason for hiding this comment

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

We also call it Tcp.bind() in Akka, but I agree something like this is better.

Just connections or source/asSource would work for me as well, this is 👍 too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just connections or source/asSource would work for me as well, this is +1 too.

All good suggestions, but lets keep connectionSource as I like the extra verbosity for that particular method.

Http().newServerAt("127.0.0.1", 443).enableHttps(https).bindFlow(commonRoutes)
Http().newServerAt("127.0.0.1", 80).bindFlow(commonRoutes)
Http().newServerAt("127.0.0.1", 443).enableHttps(https).bind(commonRoutes)
Http().newServerAt("127.0.0.1", 80).bind(commonRoutes)
Copy link
Member

Choose a reason for hiding this comment

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

👍 weird that we had bindFlow here before

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that might have been missed by scalafix for some reason.

Co-authored-by: Arnout Engelen <github@bzzt.net>
@akka-ci 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 Jul 21, 2020
@akka-ci
Copy link

akka-ci commented Jul 21, 2020

Test PASSed.

@jrudolph jrudolph changed the title Polishing and docs for ServerBuilder API core: polishing and docs for ServerBuilder API Jul 21, 2020
@jrudolph jrudolph merged commit f23a027 into akka:master Jul 21, 2020
@jrudolph jrudolph deleted the server-builder-docs branch July 21, 2020 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants