Enable SCRAM-SHA-256 and SCRAM-SHA-512 for sasl - #1918
Conversation
|
Documentation is still missing, I'll add something on Friday. |
|
@dpkp @jeffwidman ready to be reviewed now. |
|
@dpkp @jeffwidman We are extremely interested in this as well. Could you please take a look? |
|
Would be great to have this feature. Is there a timeframe for when we can merge it? Thanks! |
dpkp
left a comment
dpkp
left a comment
There was a problem hiding this comment.
Thanks so much for this PR and the excellent work on test fixtures!
| directories: | ||
| - $HOME/.cache/pip | ||
| - servers/ | ||
| - servers/dist |
| return found | ||
|
|
||
| return None | ||
| return found |
| pass | ||
|
|
||
| logging.getLogger(__name__).addHandler(NullHandler()) | ||
| logging.basicConfig(level=logging.INFO) |
There was a problem hiding this comment.
What's your thinking on the logging changes here?
| AUTHENTICATING = '<authenticating>' | ||
|
|
||
|
|
||
| class ScramClient: |
There was a problem hiding this comment.
Thoughts on moving this to its own file ?
| close = False | ||
| else: | ||
| try: | ||
| client_first = scram_client.first_message().encode() |
There was a problem hiding this comment.
should we specify an encoding explicitly? is 'utf-8' always the default? does it matter?
| self._send_bytes_blocking(size + client_first) | ||
|
|
||
| (data_len,) = struct.unpack('>i', self._recv_bytes_blocking(4)) | ||
| server_first = self._recv_bytes_blocking(data_len).decode() |
There was a problem hiding this comment.
similarly re: codec. would prefer to specific 'utf-8' explicitly
| server_first = self._recv_bytes_blocking(data_len).decode() | ||
| scram_client.process_server_first_message(server_first) | ||
|
|
||
| client_final = scram_client.final_message().encode() |
| self._send_bytes_blocking(size + client_final) | ||
|
|
||
| (data_len,) = struct.unpack('>i', self._recv_bytes_blocking(4)) | ||
| server_final = self._recv_bytes_blocking(data_len).decode() |
| if sasl_mechanism is not None: | ||
| self.sasl_mechanism = sasl_mechanism.upper() | ||
| else: | ||
| self.sasl_mechanism = None |
| # wait and try again | ||
| # on travis the brokers sometimes take a while to find themselves | ||
| time.sleep(0.5) | ||
| self._create_topic_via_admin_api(topic_name, num_partitions, replication_factor, timeout_ms) |
There was a problem hiding this comment.
might prefer an outer loop to enable as many retries as needed until timeout
|
@dpkp Hello! Will this be released soon? |
|
@dpkp @jeffwidman Any update? |
|
Alternatively, can I please do something to help? |
|
Hey @ofek I don't have the rights to cut a new release, but I just pinged @dpkp and he said on IM that he's currently traveling w/o his laptop for the next week so can't look at it until he returns. Sorry about that. If you weren't shipping to end-users and just using internally then it'd be trivial to pin to a specific commit hash rather than needing a release... but alas, you are shipping to end users. |
|
No worries, thanks! |
|
Hello again! Any update? |
|
I'm also waiting on this feature, @dpkp can you give us an update on the expected timeline? |
|
looking forward for that too ! |
|
@terence-bigtt This is released. |
8 participants
closes #1882
This change is