vapor / redis

Vapor provider for RediStack
MIT License
459 stars 58 forks source link

Auth fix #108

Closed pedantix closed 6 years ago

pedantix commented 6 years ago
pedantix commented 6 years ago

This PR should fix #105 and #107

bre7 commented 6 years ago

The issue was related to how the AUTH command was being encoded right ? The "+" that was prepended when using basicString ?

pedantix commented 6 years ago

@bre7 I am not 100% sure. The real issue is that we had an untested feature in code base ><. By adding authlinux build to the circle.yml file I had a means of debugging redis auth without doing heroic measures, after I could produce the "you aren't authed error" I was able to quickly fix the issue. Thanks to @John-Connolly 's Auth command addition I don't actually know if it was an implementation detail I just moved things around with some jiggerty pockety and ensured it was tested. But I think you are right the password was encoded in the wrong string format.

bre7 commented 6 years ago

Do I really need to accept the review or can you just merge (seeing as you do have write access) ?

vzsg commented 6 years ago

Having write access is not enough, as the master branch is protected. Only administrators can merge without an accepted review. So yeah, instead of a comment, you should leave a positive review, or wait for @tanner0101.

penny-coin commented 6 years ago

Hey @pedantix, you just merged a pull request, have a coin!

You now have 2 coins.

pedantix commented 6 years ago

@tanner0101 everytime I think I have swiftlint working again ... :D