Add context support

[johanbrandhorst]

I've aimed to keep backwards compatibility where possible, but I'd be happy to remove the functions that don't take a context if that is alright.

varlink/internal: add ctxio package

The ctxio package exposes a Conn which can support calling Read and Write with a context.

varlink: add deep context integration

Varlink uses the ctxio Conn to support writing and reading while allowing operations to be cancelled via the context.

Fixes #15

[coveralls]

Pull Request Test Coverage Report for Build 349

• 26 of 26 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.07%) to 82.865%

---

Totals
Change from base Build 340:	0.07%
Covered Lines:	590
Relevant Lines:	712

---

💛 - Coveralls

[purpleidea]

PS: How does this add waiting for events btw?

[johanbrandhorst]

PS: How does this add waiting for events btw?

Yeah it doesn't really address the original issue but it seems @haraldh reimagined it for adding context support 😅. Probably best to open a separate issue about adding context support.

[purpleidea]

SetWriteDeadline cancels any current reads. The channel already guarantees that we don't leak goroutines because it's read from before we return.

It unblocks this:

• n, err := c.conn.Write(buf)

??

[purpleidea]

It's innacurate that it "fixes #5" as in your comment. #5 is about events. My comments about context missing were to support the evidence that events didn't exist as a form of analysis.

On Mon, Oct 14, 2019 at 8:28 PM James purpleidea@gmail.com wrote:

SetWriteDeadline cancels any current reads. The channel already guarantees that we don't leak goroutines because it's read from before we return.

It unblocks this:

• n, err := c.conn.Write(buf)

??

[johanbrandhorst]

SetWriteDeadline cancels any current reads. The channel already guarantees that we don't leak goroutines because it's read from before we return. It unblocks this: + n, err := c.conn.Write(buf) ??

Yes? I updated my comment to say "writes", I wrote "reads" by mistake. See the comment on SetWriteDeadline: https://golang.org/pkg/net/#Conn.

[johanbrandhorst]

It's innacurate that it "fixes #5" as in your comment. #5 is about events. My comments about context missing were to support the evidence that events didn't exist as a form of analysis.

I've updated the description to mention issue #15 which I've just added instead.

[johanbrandhorst]

@haraldh I've aimed to keep backwards compatibility where possible, but I'd be happy to remove the functions that don't take a context if that is alright. I'm not sure what compatibility guarantees you're working with at the moment.

[johanbrandhorst]

I'm going to add context to the generated methods as well, stand by.

[johanbrandhorst]

OK I've updated the client and server side. The server side is not backward compatible, and if users are updating the varlink package, they will also need to regenerate their varlink files with the newer version of the generator.

The client side is, however, completely backwards compatible as implemented. I have tested newer versions of the varlink library with an old generated file. @haraldh I will leave it to you to decide whether we want to make a clean break here and just break existing generated clients too, so that both server side and client side users have to regenerate their files with the new interface generator version. It would allow us to remove quite a few duplicate functions. This further step could also be done in a later commit if we want.

[haraldh]

Hmm, since we never made an official go release, I hope we didn't promise anything. That said, I am fine with breaking backward compatibility.

[johanbrandhorst]

Hmm, since we never made an official go release, I hope we didn't promise anything. That said, I am fine with breaking backward compatibility.

Great, I'll remove the old functions and just add the context parameter.

[johanbrandhorst]

I've updated the PR with the breaking signature changes - I like it better this way :).

[johanbrandhorst]

I've tested the latest version of this with my podman container library: https://github.com/uw-labs/podrick/pull/6, all seems to be working fine.

[haraldh]

Thank you very much