[GitHub] [libcloud] ogayot opened a new pull request, #1690: test/test_http.py: stop HTTP server (thread) properly

[GitBox <gi...@apache.org>]

ogayot opened a new pull request, #1690:
URL: https://github.com/apache/libcloud/pull/1690

   ## test/test_http.py: stop HTTP server (thread) properly
   
   ### Description
   
   Hello,
   
   The `HttpLayerTestCase` test class creates a `HTTPServer` instance and runs it in a separate thread.
   
   After running all the test-cases, we attempt to close the server and the thread by killing the thread. Unfortunately, the code that does that is unreachable because the tear-down member function is called `tearDownCls` intead of `tearDownClass` [1].
   
   Moreover, there is no `threading.Thread.kill function`. This was undetected because the code was unreachable.
   
   The proper way to clean things up is to:
   
   1. Stop the HTTP server using `HTTPServer.shutdown()`
   2. Join the thread using `threading.Thread.join()`
   
   Thanks!
   
   [1] https://docs.python.org/3/library/unittest.html#unittest.TestCase.tearDownClass
   
   ### Status
   
   - done, ready for review
   
   ### Checklist (tick everything that applies)
   
   - [x] [Code linting](http://libcloud.readthedocs.org/en/latest/development.html#code-style-guide) (required, can be done after the PR checks)
   - [ ] Documentation
   - [ ] [Tests](http://libcloud.readthedocs.org/en/latest/testing.html) _changes are in the tests only_
   - [ ] [ICLA](http://libcloud.readthedocs.org/en/latest/development.html#contributing-bigger-changes) (required for bigger changes)
   


-- 
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@libcloud.apache.org

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

[GitHub] [libcloud] Kami commented on pull request #1690: test/test_http.py: stop HTTP server (thread) properly

[GitBox <gi...@apache.org>]

Kami commented on PR #1690:
URL: https://github.com/apache/libcloud/pull/1690#issuecomment-1122896063

   Thanks for the contribution and a good catch.
   
   The change looks good to me, but funny enough, I get this warning with your change, but I don't get it without it:
   
   ```bash
   sys:1: ResourceWarning: unclosed <socket.socket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('127.0.0.1', 57898)>
   ```
   
   Will do some more digging in. It may actually be related to outgoing client connection.


-- 
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@libcloud.apache.org

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

[GitHub] [libcloud] Kami commented on pull request #1690: test/test_http.py: stop HTTP server (thread) properly

[GitBox <gi...@apache.org>]

Kami commented on PR #1690:
URL: https://github.com/apache/libcloud/pull/1690#issuecomment-1122901211

   Added server.socket.close() and merged it into trunk, thanks!


-- 
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@libcloud.apache.org

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

[GitHub] [libcloud] asfgit merged pull request #1690: test/test_http.py: stop HTTP server (thread) properly

[GitBox <gi...@apache.org>]

asfgit merged PR #1690:
URL: https://github.com/apache/libcloud/pull/1690


-- 
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@libcloud.apache.org

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

[GitHub] [libcloud] ogayot commented on pull request #1690: test/test_http.py: stop HTTP server (thread) properly

[GitBox <gi...@apache.org>]

ogayot commented on PR #1690:
URL: https://github.com/apache/libcloud/pull/1690#issuecomment-1123283234

   > Added server.socket.close() and merged it into trunk, thanks!
   
   Oh, good catch as well @Kami, thanks!
   Looking back, I think `server.server_close()` was the expected call after `server.shutdown()`.
   
   ```python
   server.shutdown()
   server.server_close()
   ```
   
   But it seems that `server.socket.close()` calls `server.socket.close()` as you did, so I'm sure it's no big deal :)


-- 
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@libcloud.apache.org

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