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

Fix using PQs with subscriptions #6283

Merged
merged 2 commits into from
Nov 27, 2024
Merged

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Nov 26, 2024

See #6276

See test here

@martinbonnin martinbonnin requested a review from BoD as a code owner November 26, 2024 17:39
@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Nov 26, 2024

✅ Docs Preview Ready

No new or changed pages found.

@martinbonnin
Copy link
Contributor Author

Sadly the release-3.x CI has flakes. The integration tests "run on my machine" though so I'm tempted to merge anyways.

Copy link
Contributor

@BoD BoD left a comment

Choose a reason for hiding this comment

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

👍


override suspend fun url() = url ?: suspendCoroutine { cont ->
url = "http://localhost:${server.address().unsafeCast<AddressInfo>().port}/"
server.on("listening") { _ ->
val server = requireNotNull(this.server) { "Server is not listening, please call listen() before calling address()" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this.server ever be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it can't, this is a bad copy/paste from v4, I'll remove it

@martinbonnin martinbonnin merged commit d70de83 into release-3.x Nov 27, 2024
4 of 5 checks passed
@martinbonnin martinbonnin deleted the fix-pqs-with-subscriptions branch November 27, 2024 14:02
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.

3 participants