search

Thriftpy / thriftpy2

Pure python approach of Apache Thrift.
MIT License
567 stars 90 forks source link

Add sasl transport support #222

Closed ecederstrand closed 6 months ago

ecederstrand commented 1 year ago

Adds pure Python and Cython implementation of sasl transport.

ethe commented 1 year ago

Thanks, I am reviewing it.

aisk commented 1 year ago

I think this change need some test cases to ship with. And since this PR only implment the sasl transport client, maybe we can call some public thrift services with sasl support?

ecederstrand commented 1 year ago

I agree that tests would be useful. I don't know of any public thrift services with sasl support, so would really like any suggestions here, or alternative methods for testing this code. We have an internal HBase installation that we tested this against, but a full HBase is quite a task to build for a test setup.

aisk commented 1 year ago

Fully implment the sasl server transport is the best way.

Another way is starting a TCP server before running this test case, and the server can be a dummpy server which only send a valid thrift response package while accept and read fixed length of packet.

You can dump a thrift packat via wiresharkl or something like it.

aisk commented 6 months ago

Since this is a new feature and it won't break current users' code, I think we can merge this without testing, if @ecederstrand doesn't have time to finish the work.

@ethe any concerns?

ethe commented 6 months ago

Since this is a new feature and it won't break current users' code, I think we can merge this without testing, if @ecederstrand doesn't have time to finish the work.

It looks good to me, maybe we could publish a pre-release version that contains this experimental feature.