Are Bitswap Sessions actually being used?

[hannahhoward]

Possible Bug

It is not clear to me that in the current go-ipfs/master along with go-bitswap/master that any code in https://github.com/ipfs/go-bitswap/blob/master/session.go#L54 is being run.

Testing with the cat command, the call stack is as follows:
-- api.Unixfs().Get()
-- api.core().getSession()
-- dag.NewSession()
-- dagService.Session()
-- blockService.NewSession()

However, here's the issue -- to call bitswap sessions you would need to satisfy the branching condition on line 117-- but you don't and instead it just returns the value at line 125

@schomatis identified this issue in discussions of ipfs/go-bitswap#8 in relation to that feature branch of go-bitswap but it appears to affect master as well. (and can't be fixed through updating go-ipfs-exhange-interface to 0.1.1 since NewSession in master in BitSwap just returns a Session struct)

Am I missing something? Or are we not getting the benefit of sessions we intend to get.

[whyrusleeping]

@hannahhoward I remember noticing this while we were meeting in LA, some interface changes seem to have snuck in there and broken things. This is fixed in https://github.com/ipfs/go-bitswap/pull/8/files (and a type assertion was added to ensure we don't break this in the future).

We should probably apply that fix independently.

[schomatis]

This is fixed in https://github.com/ipfs/go-bitswap/pull/8/files

@whyrusleeping Where? (I mean, which line?)

[whyrusleeping]

@schomatis here: https://github.com/ipfs/go-bitswap/pull/8/files#diff-f2fccfb96064097b73fbe8eeae3703eeR70

And this assertion to ensure that it works: https://github.com/ipfs/go-bitswap/pull/8/files#diff-0e99f1e85b5e90e54545f9cf6d323debR33

[schomatis]

Oh, I understand, thanks. My original problem that @hannahhoward is citing was actually related to a version mismatch, not the interface change.

[hannahhoward]

@whyrusleeping + @Stebalien I am going to work on extracting the parts of that PR (tests + that fix) that we know we want into a seperate PR we can merge right away.

[Stebalien]

❤️

[whyrusleeping]

@hannahhoward the way i put the commits together should make it pretty easy. I separated the 'adding tests' and the 'changing sessions' into separate commits.

[hannahhoward]

Will do. I'm in TN for a conference till tomorrow but will work on this Monday.