Skip to content
This repository has been archived by the owner on Mar 24, 2021. It is now read-only.

Feature SASL SCRAM support #972

Open
wants to merge 46 commits into
base: master
Choose a base branch
from

Conversation

swenzel
Copy link

@swenzel swenzel commented Oct 7, 2019

This pull request adds support for modular SASL authentication mechanisms with two mechanisms (PLAIN, SCRAM) already implemented.

closes #651

Swen Wenzel added 30 commits October 4, 2019 11:48
@codecov-io
Copy link

codecov-io commented Oct 10, 2019

Codecov Report

Merging #972 into master will decrease coverage by 1.54%.
The diff coverage is 86.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #972      +/-   ##
==========================================
- Coverage   83.53%   81.98%   -1.55%     
==========================================
  Files          36       37       +1     
  Lines        3807     4468     +661     
  Branches      562      682     +120     
==========================================
+ Hits         3180     3663     +483     
- Misses        483      565      +82     
- Partials      144      240      +96
Impacted Files Coverage Δ
pykafka/client.py 88.46% <ø> (ø) ⬆️
pykafka/exceptions.py 100% <100%> (ø) ⬆️
pykafka/cluster.py 70.9% <100%> (+1.2%) ⬆️
pykafka/rdkafka/simple_consumer.py 87.82% <100%> (-1.56%) ⬇️
pykafka/connection.py 89.18% <100%> (+6.02%) ⬆️
pykafka/rdkafka/producer.py 91.22% <100%> (-3.32%) ⬇️
pykafka/protocol/__init__.py 100% <100%> (ø) ⬆️
pykafka/broker.py 92.22% <71.42%> (-1.42%) ⬇️
pykafka/protocol/sasl.py 84.93% <84.93%> (ø)
pykafka/sasl_authenticators.py 85.03% <85.03%> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7665bf...73f8e8e. Read the comment docs.

@swenzel
Copy link
Author

swenzel commented Oct 10, 2019

Ditched kafka 0.8 because there were too many issues getting it to run. It neither supports SSL nor SASL, the configs differ significantly and there seems to be an issue with its zk client. I thought it's more useful to test kafka 2 instead.

Switched to librdkafka 0.11.5 because 0.9.5 doesn't support the SCRAM mechanisms. In the end I didn't choose a 1.x version since it introduces another configuration issue that I didn't want to solve.

I went far out of scope fixing several issues with CI and the tests which took me a lot longer than actually implementing the feature.
I've really put a lot of work into it but now is the point where I say it's enough. I'm open to refactoring certain things or to revert a few changes if you don't want them. But I won't continue fixing CI or other tests that do not belong to the code that I changed. Especially if it's for keeping python 2 or kafka 0 compatibility.

@hfjn
Copy link

hfjn commented Nov 19, 2019

Could someone please just take a look at this PR? I think @swenzel has done some great work here which really pushes pykafka forward at should be a high priority to work on.

@garrettthomaskth
Copy link

Hi, I would also like to see this merged! :D

@Neustradamus
Copy link

Any news?

@DoZX
Copy link

DoZX commented Aug 21, 2020

Look forward to this feature.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SASL Support
6 participants