Connection interface

[nemoshlag]

Suggested PR for #91. This enables OpAMP to close the network connection to an agent from the server side

[tigrannajaryan]

@andykellr please take a look, I believe you wanted this capability.

[codecov[bot]]

Codecov Report

Base: 74.61% // Head: 74.74% // Increases project coverage by +0.12% :tada:

Coverage data is based on head (daa8363) compared to base (89eeb30). Patch coverage: 83.33% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
+ Coverage   74.61%   74.74%   +0.12%     
==========================================
  Files          23       23              
  Lines        1765     1770       +5     
==========================================
+ Hits         1317     1323       +6     
+ Misses        334      333       -1     
  Partials      114      114              

Impacted Files	Coverage Δ
server/httpconnection.go	33.33% <75.00%> (+33.33%)	:arrow_up:
server/wsconnection.go	70.00% <100.00%> (+7.50%)	:arrow_up:
server/callbacks.go	72.72% <0.00%> (+4.54%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[andykellr]

@andykellr please take a look, I believe you wanted this capability.

This will be a good addition. I agree with your feedback.

The http Close() is a strange case. Due to the short-lived nature of the http connection, Close() would only serve to interrupt the current message exchange in a single request and could only be called from OnConnected or OnMessage unless the connection were preserved in one of those methods and called from a separate goroutine.

Is there a need for Close() in http? If not, should we leave the http implementation empty? Would it be preferable to rename the method to Disconnect() or something else that better reflects the intent of closing a persistent connection to the server?

[tigrannajaryan]

I think Disconnect() is a better name and it is fine to make it an error to call Disconnect() for plain http connections.

[andykellr]

I think Disconnect() is a better name and it is fine to make it an error to call Disconnect() for plain http connections.

I was going to suggest that it be a no-op, but an error sound good. I would suggest creating an error like ErrDisconnectHttpConnection or something similar that can be easily checked and ignored by the caller in the event that the calling code is unaware of the type of connection be used.

[tigrannajaryan]

I think Disconnect() is a better name and it is fine to make it an error to call Disconnect() for plain http connections.

I was going to suggest that it be a no-op, but an error sound good. I would suggest creating an error like ErrDisconnectHttpConnection or something similar that can be easily checked and ignored by the caller in the event that the calling code is unaware of the type of connection be used.

Yes, sorry, I want's clear. I meant it is good to return an error from Disconnect(), wasn't suggesting that it panics or anything.

[tigrannajaryan]

@andykellr please take another look and if it looks good please merge.

[tigrannajaryan]

Merged. Thanks @nemoshlag!