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 2020/06/03 10:37:02 UTC

[GitHub] [thrift] phile314 opened a new pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

phile314 opened a new pull request #2169:
URL: https://github.com/apache/thrift/pull/2169


   Upgrade to network 3.0.0.0. Note that the `Network` module has been removed in version 3.0.0.0 and hence the thrift library no longer compiled with the current network package.
   
   This is a breaking change, but it should be fairly easy to update all call sites.
   
   <!-- Explain the changes in the pull request below: -->
     
   
   <!-- We recommend you review the checklist/tips before submitting a pull request. -->
   
   - [x] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket?  (not required for trivial changes)
   - [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)
   - [ ] Did you do your best to avoid breaking changes?  If one was needed, did you label the Jira ticket with "Breaking-Change"?
   - [ ] If your change does not involve any code, include `[skip ci]` anywhere in the commit message to free up build resources.
   
   <!--
     The Contributing Guide at:
     https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
     has more details and tips for committing properly.
   -->
   


----------------------------------------------------------------
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.

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



[GitHub] [thrift] stale[bot] commented on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2169:
URL: https://github.com/apache/thrift/pull/2169#issuecomment-674344768


   This issue has been automatically closed due to inactivity. Thank you for your contributions.
   


----------------------------------------------------------------
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.

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



[GitHub] [thrift] phile314 commented on a change in pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

Posted by GitBox <gi...@apache.org>.
phile314 commented on a change in pull request #2169:
URL: https://github.com/apache/thrift/pull/2169#discussion_r434472569



##########
File path: lib/hs/src/Thrift/Server.hs
##########
@@ -42,25 +42,41 @@ runThreadedServer :: (Protocol i, Protocol o)
                   => (Socket -> IO (i, o))
                   -> h
                   -> (h -> (i, o) -> IO Bool)
-                  -> PortID
+                  -> Int
                   -> IO a
 runThreadedServer accepter hand proc_ port = do
-    socket <- listenOn port
-    acceptLoop (accepter socket) (proc_ hand)
+    let hints = defaultHints {

Review comment:
       This is basically copy-paste from the network library how-to on how to listen on a socket.

##########
File path: lib/hs/src/Thrift/Transport/Handle.hs
##########
@@ -62,17 +63,12 @@ instance Transport Handle where
     tWrite = LBS.hPut
     tFlush = hFlush
 
-
--- | Type class for all types that can open a Handle. This class is used to
--- replace tOpen in the Transport type class.
-class HandleSource s where

Review comment:
       This class does not look really useful. I have removed it, but kept a `hOpenSocket` function to make the common use case of connecting to a socket concise.

##########
File path: lib/hs/thrift.cabal
##########
@@ -32,19 +32,12 @@ License-File:   LICENSE
 Description:
   Haskell bindings for the Apache Thrift RPC system. Requires the use of the thrift code generator.
 
-flag network-uri
-   description: Get Network.URI from the network-uri package
-   default: True
-
 Library
   Hs-Source-Dirs:
     src
   Build-Depends:
-    base >= 4, base < 5, containers, ghc-prim, attoparsec, binary, bytestring >= 0.10, base64-bytestring, hashable, HTTP, text, hspec-core > 2.4.0, unordered-containers >= 0.2.6, vector >= 0.10.12.2, QuickCheck >= 2.8.2, split
-  if flag(network-uri)
-     build-depends: network-uri >= 2.6, network >= 2.6 && < 3.0
-  else
-     build-depends: network < 2.6

Review comment:
       The network/network-uri split has already been a few years in the past, so I think it is safe to drop that compatibility code.




----------------------------------------------------------------
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.

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



[GitHub] [thrift] phile314 edited a comment on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

Posted by GitBox <gi...@apache.org>.
phile314 edited a comment on pull request #2169:
URL: https://github.com/apache/thrift/pull/2169#issuecomment-638234535


   Hm, I am completely confused now if I should use cmake or configure && make. There seems to be a cmake file inside `test/hs`, but the instructions in the top-level README refer to make. Can anybody help me?
   
   EDIT: Ah oops, so it is cmake && make.


----------------------------------------------------------------
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.

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



[GitHub] [thrift] stale[bot] commented on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2169:
URL: https://github.com/apache/thrift/pull/2169#issuecomment-670813116


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.
   


----------------------------------------------------------------
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.

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



[GitHub] [thrift] phile314 commented on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

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


   I noticed now that this broke the cross-language haskell tests. I tried to get those tests running locally, but it looks like running them using a recent GHC/cabal version breaks a lot of things ....
   
   Not sure what the best thing to do is ....


----------------------------------------------------------------
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.

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



[GitHub] [thrift] phile314 commented on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

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


   Hm, I am completely confused now if I should use cmake or configure && make. There seems to be a cmake file inside `test/hs`, but the instructions in the top-level README refer to make. Can anybody help me?


----------------------------------------------------------------
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.

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



[GitHub] [thrift] zeshuai007 commented on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

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


   Hi @phile314 ,both `cmake` and `make` can work, you can choose your favorite build method.But it seems that `cmake` is gradually becoming a mainstream build method.


----------------------------------------------------------------
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.

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



[GitHub] [thrift] phile314 edited a comment on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

Posted by GitBox <gi...@apache.org>.
phile314 edited a comment on pull request #2169:
URL: https://github.com/apache/thrift/pull/2169#issuecomment-638219061


   I noticed now that this broke the cross-language haskell tests. I tried to get those tests running locally, but it looks like running them using a recent GHC/cabal version breaks a lot of things related to the test build system ....
   
   Not sure what the best thing to do is ....


----------------------------------------------------------------
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.

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



[GitHub] [thrift] zeshuai007 edited a comment on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

Posted by GitBox <gi...@apache.org>.
zeshuai007 edited a comment on pull request #2169:
URL: https://github.com/apache/thrift/pull/2169#issuecomment-638548876


   Hi @phile314 ,both `cmake` and `configure && make` can work, you can choose your favorite build method.But it seems that `cmake` is gradually becoming a mainstream build method.


----------------------------------------------------------------
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.

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



[GitHub] [thrift] phile314 commented on pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

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


   The tests are passing now, this is ready for review.


----------------------------------------------------------------
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.

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



[GitHub] [thrift] stale[bot] closed pull request #2169: THRIFT-4731: Replace usage of deprecated network modules in haskell library

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #2169:
URL: https://github.com/apache/thrift/pull/2169


   


----------------------------------------------------------------
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.

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