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

Added support for listening to notifications #52

Closed
wants to merge 1 commit into from

Conversation

rdegnan
Copy link
Contributor

@rdegnan rdegnan commented Jan 9, 2019

It seems there is very little in the way of exposing Postgres asynchronous notifications (https://www.postgresql.org/docs/9.3/libpq-notify.html) via the client. This allows a consumer to be configured which is called each time a NotificationResponse is received. This allows r2dbc users to take advantage of basic pub/sub functionality via the Postgres LISTEN/NOTIFY mechanism (see tests for examples).

In terms of the API I'm not sure whether a Consumer is preferable or if it would be better to expose a Flux directly.

@@ -258,6 +267,11 @@ public TransactionStatus getTransactionStatus() {
return this.transactionStatus.get();
}

@Override
public Disposable addNotificationListener(Consumer<NotificationResponse> consumer) {
Copy link

Choose a reason for hiding this comment

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

Isn't it better to expose Flux<NotificationResponse> here?

@nebhale nebhale self-assigned this Jan 14, 2019
@nebhale nebhale added the type: enhancement A general enhancement label Jan 14, 2019
@nebhale
Copy link
Contributor

nebhale commented Jan 14, 2019

I'll take a look and provide some feedback, but this will be an M8 contribution, in line with discussion on the mailing list.

@nebhale nebhale added the status: pending-design-work Needs design work before any code can be developed label Jan 24, 2019
@nebhale nebhale added this to the 1.0.0.M8 milestone Feb 1, 2019
@nebhale nebhale removed this from the 0.8.0.M8 milestone May 10, 2019
@mp911de mp911de assigned mp911de and unassigned nebhale Sep 6, 2019
@mp911de
Copy link
Collaborator

mp911de commented Sep 6, 2019

Sorry for the long radio silence. It took us a while to sort out a few issues in the R2DBC project.

With r2dbc/r2dbc-spi#35 we're envisioning a standardized approach for event providers (such as connection events or Pub/Sub notifications). It would make sense to expose Flux<Notification> that multicasts events via DirectProcessor or a similar processor to subscribers.

What do you think? As this PR is here for quite some time, we should rebase it if the suggestion above makes sense.

@mp911de mp911de removed the status: pending-design-work Needs design work before any code can be developed label Sep 6, 2019
@mp911de mp911de added this to the 0.8.0.RC1 milestone Sep 10, 2019
@mp911de mp911de closed this in da81933 Sep 10, 2019
mp911de added a commit that referenced this pull request Sep 10, 2019
Expose notification events as Flux<Notification>.

Connection sender, receiver = …;

Flux<Notification> listen = receiver.createStatement("LISTEN mymessage").execute()
                                .flatMap(PostgresqlResult::getRowsUpdated)
                                .thenMany(receiver.getNotifications());

Mono<Void> notify = sender.createStatement("NOTIFY mymessage, 'Hello World'").execute().flatMap(PostgresqlResult::getRowsUpdated).then()

[#52]
@mp911de
Copy link
Collaborator

mp911de commented Sep 10, 2019

Thanks a lot. That's merged and polished now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants