kreds icon indicating copy to clipboard operation
kreds copied to clipboard

Subscriber Authentication

Open rexlManu opened this issue 2 years ago • 11 comments

I tried to use the subscriber client with a redis instance thats password-protected and found out by #8 that authentication is missing.

rexlManu avatar Jul 03 '22 01:07 rexlManu

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

sonarcloud[bot] avatar Jul 03 '22 01:07 sonarcloud[bot]

@crackthecodeabhi can you review this PR and make a patch release for it? I'm in desperate need for this for my project and Kreds would be perfect (for my current use case) if this feature was added. Thanks in advance :)

TomTruyen avatar Jul 23 '22 09:07 TomTruyen

@crackthecodeabhi can you review this PR and make a patch release for it? I'm in desperate need for this for my project and Kreds would be perfect (for my current use case) if this feature was added. Thanks in advance :)

You can download my fork and install it in your local maven repository, if you need it urgent. I think the author doesn't maintain it further.

rexlManu avatar Jul 23 '22 09:07 rexlManu

@crackthecodeabhi can you review this PR and make a patch release for it? I'm in desperate need for this for my project and Kreds would be perfect (for my current use case) if this feature was added. Thanks in advance :)

You can download my fork and install it in your local maven repository, if you need it urgent. I think the author doesn't maintain it further.

I have just been in contact with the author and he said the following: Yes, I will review and add some test cases to it before making a release. Should be done in a week.

But I will probably do what you said until the new release is available

TomTruyen avatar Jul 23 '22 09:07 TomTruyen

@rexlManu @TomTruyen will be reviewing and merging the issue ASAP. been busy lately. Give me sometime.

crackthecodeabhi avatar Jul 23 '22 09:07 crackthecodeabhi

@rexlManu @TomTruyen will be reviewing and merging the issue ASAP. been busy lately. Give me sometime.

Is there an ETA on this? The PubSub functionality is almost useless without it... I am stuck on project because of this issue

JefCodes avatar Jul 30 '22 14:07 JefCodes

@rexlManu @TomTruyen will be reviewing and merging the issue ASAP. been busy lately. Give me sometime.

Is there an ETA on this? The PubSub functionality is almost useless without it... I am stuck on project because of this issue

There needs to be some work done on this PR, the current PR will cause a dead lock since the reader coroutine is not pre-empted before writing on the connection.

Will update as soon i fix it and run some tests.

crackthecodeabhi avatar Jul 30 '22 16:07 crackthecodeabhi

@rexlManu @TomTruyen will be reviewing and merging the issue ASAP. been busy lately. Give me sometime.

Is there an ETA on this? The PubSub functionality is almost useless without it... I am stuck on project because of this issue

There needs to be some work done on this PR, the current PR will cause a dead lock since the reader coroutine is not pre-empted before writing on the connection.

Will update as soon i fix it and run some tests.

I didn't get a dead lock when using @rexlManu his version. Does the dead lock only happen in specific scenario's?

TomTruyen avatar Aug 12 '22 14:08 TomTruyen

I'm currently using my version pretty much and didn't run in any issues. Maybe share a example where it occurs.

rexlManu avatar Aug 12 '22 20:08 rexlManu

@rexlManu the subscriber coroutine suspends [waititng to read messages from redis] after it locks the mutex on the Konnection. the Connection command interface which has now been added as implements to Subscriber Client, will not preempt this coroutine, before it takes a lock on the that connection mutex.

if the user subscribes to a channel, and then tries to execute any connection command, it will block indefinitely.

Hence, a clean approach would be to , accept optional auth parameters while building the subscriber client, and authenticate against redis before doing anything else, this ensure all other commands work as usual with no change.

also correction on my part, it won't be a dead lock but unintended blocking of the coroutine indefinitely.

crackthecodeabhi avatar Aug 18 '22 10:08 crackthecodeabhi

@rexlManu @TomTruyen will be reviewing and merging the issue ASAP. been busy lately. Give me sometime.

Is there an ETA on this? The PubSub functionality is almost useless without it... I am stuck on project because of this issue

There needs to be some work done on this PR, the current PR will cause a dead lock since the reader coroutine is not pre-empted before writing on the connection. Will update as soon i fix it and run some tests.

I didn't get a dead lock when using @rexlManu his version. Does the dead lock only happen in specific scenario's?

After calling the subscribe command on the subscriber client, call any other connection command, the coroutine will block

crackthecodeabhi avatar Aug 18 '22 10:08 crackthecodeabhi