async context manager and test

[nathan-chappell]

Signed-off-by: Nathan Chappell [email protected]

Enhancement / Feature

Currently, we would like to use the python-vcx bindings in a server environment. The server must operate in an async-hronous fashion, and in some cases needs to use libvcx. While spawning a new process to completely isolate all libvcx operations would be ideal, it is unfortunately not an option for us. Since libvcx can only "manage one connection at a time" (see comment below), we require synchronization between async processes. This has led to the need to use the idiom:

await synchronization mechanism
await libvcx initialization
# ... do work
libvcx shutdown
release synchronization mechanism

This seems like the type of thing that should be a part of the libraries underlying utilities, here are a few reasons why:

• it's an obvious pattern whose logic can be consolidated
• the library implementers can add or remove any additional logic that is required / deprecated in the future
• it documents / communicates the semantics for multiple-uses of the library at the same time

Comments

I'm no domain-expert, nor do I have a lot of experience with this library. The phrasing "manage one connection at a time" may be erroneous, but the intent should be clear. Also, it may be that additional logic is required for these context managers to be fully functional, we're exploring options with our current implementation.

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 7d223f5b45c097b7033bc88f651c907b8b6a996d into 7f4fbb055478181761347a981544fb823d6f47ba - view on LGTM.com

new alerts:

• 1 for Unused import

[nathan-chappell]

I don't know why jenkins failed - none of my code touched anything else. I did notice an issue while running tests though - the python tests for get_version were throwing SEGFAULTS and killing everything. The implementation seems to use ctypes improperly and dereferences an invalid pointer. If that's the case, I can put in a correction to the python code, but I don't know what the desired behavior of get_version should be...

Currently, there's a bug in the implementation. While the rust-api for get_version indicates that a *const c_char is returned from a call to vcx_version, the python wrapper do_call_sync automatically assumes every return value from the library is an integer. This leads to problems when reporting errors, and eventually leads to a SEGFAULT when the python-function get_version tries to cast the integer into a c_char_p. The restype needs to be specified before calling the foreign function - see this stack overflow post for details.

At any rate, I could fix this, but I would just make the do_call and do_call_sync functions behave differently depending on the return type of the rust library... Maybe there is a better way, but that's the most direct way.