#44 mllp client: read full response from the server

[cezio]

fix #44

[johnpaulett]

Thanks for the contribution -- the client was mainly written to provide simple testing via the mllp_send command, so it could use some work to be more "production-ready".

• The addition of duration feels odd to me -- it means even a fast responding server that sends a single response will take a minimum of 3 seconds until the client returns the result, right? Not fully thought thru, but should the receiving of the messages instead become more aware of the LLP protocol and receive until it receives the end of message?
• timeout seems reasonable. 👍
• We'll want test coverage for the changes (but I think the duration potentially needs to be reworked)
• I'd suggest removing the logging.debug -- I get it was probably useful when developing this PR, but don't think it adds much in real world running, since the received data will just be returned.

[cezio]

Thanks for the contribution -- the client was mainly written to provide simple testing via the mllp_send command, so it could use some work to be more "production-ready".

In most cases I use MLLPClient class directly, but mllp_send command is an invaluable friend in some rescue-the-world scenarios.

• The addition of duration feels odd to me -- it means even a fast responding server that sends a single response will take a minimum of 3 seconds until the client returns the result, right? Not fully thought thru, but should the receiving of the messages instead become more aware of the LLP protocol and receive until it receives the end of message?

It's application-level deadline for the ack receiving operation. I had to split this from socket-level timeout because of the way the client is used inside my code. But yes, you're right it would wait duration time even if the ACK was already received in full. I've added a simple check for this case.

• timeout seems reasonable. +1
• We'll want test coverage for the changes (but I think the duration potentially needs to be reworked)

Yep, tests from test_client.py were failing. Sorry, my bad. I didn't check them in the first run.

I've updated tests to follow changes in client code and added a test for end-of-message bytes check.

• I'd suggest removing the logging.debug -- I get it was probably useful when developing this PR, but don't think it adds much in real world running, since the received data will just be returned.

If you use mllp_client command, then it may looks redundant. However, when it's below few layers of code, you can control this with a proper logging setup of the application.

I've added a condition to set appropriate logging levels depending on verbose flag when mllp_send is called. Would that work for you?

[NiklasMM]

I think we might be experiencing this issue as well. Any chance to get this PR finished? I'm happy to help.

[asafsilman]

@johnpaulett Any update on this PR, we are also waiting for this feature to be implemented as it would be incredibly handy for our testing.

[LukasKlement]

Could this PR be merged or are there any blockers left?