4 ms·

Seems like a context manager is what you really want, right? some other exception could be thrown from some deeper frame while sock is in scope and this wouldn

Seems like a context manager is what you really want, right? some other exception could be thrown from some deeper frame while sock is in scope and this wouldn't ensure it gets closed.

smnrchrds 5y ago
Or a "finally" block.
- postpawl 5y ago
  The sock.close() can also throw it’s own OSError, so it needs its own try/except block. For example, this wouldn’t handle the OSError on close: try: sock.shutdown() except OSError: pass finally: sock.close()
  - _ZeD_ 5y ago
    try: try: sock.shutdown() finally: sock.close() except OSError: pass Or, given the .close method from contextlib import closing try: with closing(sock): sock.shutdown() except OSError: pass
    - masklinn 5y ago
      `contextlib.ignore` could also be useful here e.g. with contextlib.ignore(OSError),\ closing(sock): sock.shutdown()
      - heavyset_go 5y ago
        contextlib.ignore was renamed to contextlib.suppress
      - dncornholio 5y ago
        There is nothing wrong with the proposed solution though? I think you are over doing it IMHO, creating unnecessary complexion.
        masklinn 5y ago
        I’ve no idea what you’re to trying to express. My comment does not imply there’s anything wrong with the comment, it just uses a tool from the standard library to reduce nesting and (as far as i’m concerned) make it more explicit and upfront. And support the original query.
        _ZeD_ 5y ago
        well, the nested try: ... finally: gave you something to think about: why do you have a "sock" object that you need to explicitly close "manually"? I didn't look at the source, it can be for a number of reasons. But I suspect some long lived sock object shared between functions. Something that could be wrapped in a context manager. like with MyCtxMgr() as my_ctx_mgr: # open the socket, init, etc... my_ctx_mgr.do_stuff() # internally it has access my_ctx_mgr.do_another_stuff() # to the opened socket external_function(my_ctx_mgr.get_sock()) # and can pass it # in the __exit__ actually close the socket, dealing with the OSError calling a barebone "obj.close()" is a code smell IMHO
      - deleted 5y ago
        [deleted]
postpawl 5y ago
As far as I know, the only errors that you're supposed to "except: pass" from socket.shutdown are OSErrors. I think you want anything else (if it even throws anything else?) to be fatal. This is the same way cpython handles it: https://github.com/python/cpython/blob/b56774bd93822e1598bb5d6c1d0b1fe8c45a750d/Lib/socketserver.py#L509-L514 https://github.com/python/cpython/blob/b56774bd93822e1598bb5... The context manager idea sounds good, but I think that might require a lot of refactoring.

Seems like a context manager is what you really want, right? some other exception could be thrown from some deeper frame while sock is in scope and this wouldn't ensure it gets closed.

smnrchrds 5y ago

Or a "finally" block.

postpawl 5y ago
The sock.close() can also throw it’s own OSError, so it needs its own try/except block. For example, this wouldn’t handle the OSError on close: try: sock.shutdown() except OSError: pass finally: sock.close()
- _ZeD_ 5y ago
  try: try: sock.shutdown() finally: sock.close() except OSError: pass Or, given the .close method from contextlib import closing try: with closing(sock): sock.shutdown() except OSError: pass
  - masklinn 5y ago
    `contextlib.ignore` could also be useful here e.g. with contextlib.ignore(OSError),\ closing(sock): sock.shutdown()
    - heavyset_go 5y ago
      contextlib.ignore was renamed to contextlib.suppress
    - dncornholio 5y ago
      There is nothing wrong with the proposed solution though? I think you are over doing it IMHO, creating unnecessary complexion.
      - masklinn 5y ago
        I’ve no idea what you’re to trying to express. My comment does not imply there’s anything wrong with the comment, it just uses a tool from the standard library to reduce nesting and (as far as i’m concerned) make it more explicit and upfront. And support the original query.
      - _ZeD_ 5y ago
        well, the nested try: ... finally: gave you something to think about: why do you have a "sock" object that you need to explicitly close "manually"? I didn't look at the source, it can be for a number of reasons. But I suspect some long lived sock object shared between functions. Something that could be wrapped in a context manager. like with MyCtxMgr() as my_ctx_mgr: # open the socket, init, etc... my_ctx_mgr.do_stuff() # internally it has access my_ctx_mgr.do_another_stuff() # to the opened socket external_function(my_ctx_mgr.get_sock()) # and can pass it # in the __exit__ actually close the socket, dealing with the OSError calling a barebone "obj.close()" is a code smell IMHO
    - deleted 5y ago
      [deleted]

postpawl 5y ago

As far as I know, the only errors that you're supposed to "except: pass" from socket.shutdown are OSErrors. I think you want anything else (if it even throws anything else?) to be fatal. This is the same way cpython handles it: https://github.com/python/cpython/blob/b56774bd93822e1598bb5d6c1d0b1fe8c45a750d/Lib/socketserver.py#L509-L514 https://github.com/python/cpython/blob/b56774bd93822e1598bb5... The context manager idea sounds good, but I think that might require a lot of refactoring.