You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2022/03/11 01:37:40 UTC

[GitHub] [thrift] tokcum opened a new pull request #2545: THRIFT-5283: add support for Unix Domain Sockets in lib/rs

tokcum opened a new pull request #2545:
URL: https://github.com/apache/thrift/pull/2545


   Client: rs
   
   The Thrift crate does not support Unix Domain Sockets (UDS). After reviewing and expressing my thoughts on THRIFT-5283, I added support for UDS.
   
   This is not a breaking change. Inspired by how actix_web::HttpServer supports UDS, I added an additional fn to listen to UDS. I reorganized some other existing code to be able to reuse it for UDS. I also had to implement the trait TIoChannel for UnixStream. This went to transport::socket.
   
   As UnixStream is only available on Unix, I tagged the code with #[cfg(unix)]. I also cross compiled lib/rs to Windows with the following cargo targets:
   
       i686-pc-windows-gnu
       i686-pc-windows-msvc
       x86_64-pc-windows-gnu
       x86_64-pc-windows-msvc
   
   Last but not least I ran the rust <-> rust test harnesses as well as the cross test harnesses. This looks good. At least, my contribution didn't introduce new fails.
   
   - [x] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket?
   -> already existed as THRIFT-5283
   - [x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
   - [x] Did you squash your changes to a single commit?  (not required, but preferred)
   - [x] Did you do your best to avoid breaking changes?
   -> not a breaking change.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [thrift] allengeorge merged pull request #2545: THRIFT-5283: add support for Unix Domain Sockets in lib/rs

Posted by GitBox <gi...@apache.org>.
allengeorge merged pull request #2545:
URL: https://github.com/apache/thrift/pull/2545


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [thrift] allengeorge commented on pull request #2545: THRIFT-5283: add support for Unix Domain Sockets in lib/rs

Posted by GitBox <gi...@apache.org>.
allengeorge commented on pull request #2545:
URL: https://github.com/apache/thrift/pull/2545#issuecomment-1082850013


   @tokcum Sorry for the delay, and thank you so much for your contribution!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [thrift] tokcum commented on pull request #2545: THRIFT-5283: add support for Unix Domain Sockets in lib/rs

Posted by GitBox <gi...@apache.org>.
tokcum commented on pull request #2545:
URL: https://github.com/apache/thrift/pull/2545#issuecomment-1071756682


   Actually this PR is a blocker for my osquery-rust crate. I appreciate any feedback on how to bring this PR forward.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [thrift] tokcum commented on pull request #2545: THRIFT-5283: add support for Unix Domain Sockets in lib/rs

Posted by GitBox <gi...@apache.org>.
tokcum commented on pull request #2545:
URL: https://github.com/apache/thrift/pull/2545#issuecomment-1071756682


   Actually this PR is a blocker for my osquery-rust crate. I appreciate any feedback on how to bring this PR forward.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [thrift] tokcum commented on pull request #2545: THRIFT-5283: add support for Unix Domain Sockets in lib/rs

Posted by GitBox <gi...@apache.org>.
tokcum commented on pull request #2545:
URL: https://github.com/apache/thrift/pull/2545#issuecomment-1072536776


   > Looks good! Suggest replacing use of comparison with "" with `Option` and `if let` for the UDS usages.
   > 
   > Thank you!
   
   Thanks Allen, will do and update PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [thrift] tokcum commented on pull request #2545: THRIFT-5283: add support for Unix Domain Sockets in lib/rs

Posted by GitBox <gi...@apache.org>.
tokcum commented on pull request #2545:
URL: https://github.com/apache/thrift/pull/2545#issuecomment-1068429337


   I tried to figure out why one of the Travis CI jobs failed. To be honest, I've no clue. I do not see a connection to the PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [thrift] tokcum commented on pull request #2545: THRIFT-5283: add support for Unix Domain Sockets in lib/rs

Posted by GitBox <gi...@apache.org>.
tokcum commented on pull request #2545:
URL: https://github.com/apache/thrift/pull/2545#issuecomment-1072876610


   > Looks good! Suggest replacing use of comparison with "" with `Option` and `if let` for the UDS usages.
   > 
   > Thank you!
   
   Surprisingly the linter discouraged me to use if let and pointed me to .is_none() just to complain again and proposing a match statement. This didn't happen when I compiled with cargo build but when I used make to run the tests. It looks like additional rules apply depending on which tools are used.
   
   Just for reference, the two messages I received where:
   
   ```
   error: redundant pattern matching, consider using `is_none()`
   
   error: You checked before that `unwrap()` cannot fail. Instead of checking and unwrapping, it's better to use `if let` or `match`.
   ```
   
   So I used the match statement.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org