Skip to content

Replace Postgresql with Redis - #210

Merged
DanGould merged 1 commit into
payjoin:masterfrom
DanGould:redis
Mar 13, 2024
Merged

Replace Postgresql with Redis#210
DanGould merged 1 commit into
payjoin:masterfrom
DanGould:redis

Conversation

@DanGould

Copy link
Copy Markdown
Member

Close #131
Close #209 as a side effect

@DanGould
DanGould requested a review from jbesraa March 12, 2024 15:02
Comment thread payjoin-directory/src/db.rs Outdated
Comment thread payjoin-directory/src/db.rs Outdated
// Since the stream is dropped here, we can now unsubscribe
pubsub_conn.unsubscribe(&channel_name).await?;

Ok(data)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think its important testing the database implementation

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

there aren't unit tests, but the integration tests do test all functions including async peek.

Comment thread payjoin-directory/src/db.rs Outdated
Comment thread payjoin-directory/src/db.rs Outdated
let mut conn = self.client.get_async_connection().await?;
let key = format!("{}:{}", pubkey_id, channel_type);
conn.set(&key, data.clone()).await?;
conn.publish(&key, data).await?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is confusing to me. if we always set the data, why wouldnt we only get it ? we do we also need to publish and subscribe?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nice detail oriented catch, this can push a unit value or a simple "updated" string instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe its worth writing a test to demonstrate the different use cases?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

which use cases that aren't tested? The only one I reckon is polling which may be done with a near-future payjoin-io crate.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I am mainly confused about when the pub/sub will be utilized rather than the set/get. maybe iam missing something of how this should work in practice

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

if the receiver is waiting for a payjoin psbt, it makes a GET request to the endpoint which calls peek. If there's no psbt yet then the redis::get will fail, but it'll subscribe for an update. If before the timeout ends the sender comes and posts a psbt which is redis::set, it'll also publish an event which the receiver is listening for. The receiver knows to redis::get again with a fresh value.

@DanGould
DanGould merged commit 14735f4 into payjoin:master Mar 13, 2024
@DanGould
DanGould deleted the redis branch March 13, 2024 00:12
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.

Build payjoin-directory with msrv 1.63.0 Prefer Redis to Postgres for payjoin-relay

2 participants