Skip to content

Terminate Notification stream on disconnect #227

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

Merged
merged 1 commit into from
Jan 28, 2020
Merged

Conversation

mp911de
Copy link
Collaborator

@mp911de mp911de commented Jan 20, 2020

We now terminate the notification stream (PostgresqlConnection.getNotifications()) when the connection gets disconnected. If the connection is closed normally, the stream terminates successfully (onComplete). Unintended disconnects result in an error (onError).

Requires forward-port to master and 0.8.x..

[#212]

@mp911de mp911de added this to the 0.8.1.RELEASE milestone Jan 20, 2020
@gregturn
Copy link
Collaborator

@mp911de I have tried rebasing this against latest 0.8.x but there are too many changes to ReactorNettyClient that I don't 100% grok.

@gregturn gregturn force-pushed the NotificationTermination branch from e12769f to d29d780 Compare January 27, 2020 22:29
@gregturn
Copy link
Collaborator

I rebased the PR. After cleaning up ReactorNettyClient, it still doesn't compile due to a new static class now pointed at a non-static field. It also has two versions of exchange(), neither of which I can discern is the "correct" one. (Perhaps the correct version is a combination of the two?)

If you can fix it, then perhaps rebasing/squashing is in order.

We now terminate the notification stream (PostgresqlConnection.getNotifications()) when the connection gets disconnected. If the connection is closed normally, the stream terminates successfully (onComplete). Unintended disconnects result in an error (onError).

[#212]
@mp911de mp911de force-pushed the NotificationTermination branch from d29d780 to 68ebc2a Compare January 28, 2020 08:15
@mp911de mp911de changed the base branch from 0.8.x to master January 28, 2020 08:16
mp911de added a commit that referenced this pull request Jan 28, 2020
@mp911de mp911de merged commit 68ebc2a into master Jan 28, 2020
@mp911de mp911de deleted the NotificationTermination branch January 28, 2020 08:16
@mp911de mp911de added the type: bug A general bug label Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants