Tech Debt: Remove IPLD Bridge #55
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove the encode and decode on ipldbridge as part of ramp up to removing entirely Also move selector encode/decode to within protobuf encoding
Removed mock selector spec, switch to using more real blockchain simulator
remove remaining methods for ipld bridge
Remove all code references to a "bridge", consolodate test utils
check for nil selector in a request and error if it is present
Stebalien
reviewed
Mar 23, 2020
| github.com/ipfs/go-cid v0.0.3 | ||
| github.com/ipfs/go-datastore v0.0.5 | ||
| github.com/ipfs/go-ipfs-blockstore v0.0.1 | ||
| github.com/ipfs/go-blockservice v0.1.3-0.20190908200855-f22eea50656c |
There was a problem hiding this comment.
the issue here is that go-blockservice 1.3 uses go-datastore 0.4.4. Currently we are not compatible with datastore 0.4.4 in filecoin land. I think for these dependency updates I'd like to wait for the 0.5.0 IPFS release and then do them in one chunk, with the official dependencies of go-ipfs 0.5.0
| github.com/ipfs/go-datastore v0.0.5 | ||
| github.com/ipfs/go-ipfs-blockstore v0.0.1 | ||
| github.com/ipfs/go-blockservice v0.1.3-0.20190908200855-f22eea50656c | ||
| github.com/ipfs/go-cid v0.0.4-0.20191112011718-79e75dffeb10 |
| github.com/ipfs/go-ipld-format v0.0.2 | ||
| github.com/ipfs/go-log v0.0.1 | ||
| github.com/ipfs/go-merkledag v0.2.3 | ||
| github.com/ipfs/go-log v1.0.0 |
| github.com/btcsuite/btcd v0.0.0-20190629003639-c26ffa870fd8 // indirect | ||
| github.com/gogo/protobuf v1.2.1 | ||
| github.com/golang/protobuf v1.3.2 // indirect | ||
| github.com/filecoin-project/go-data-transfer v0.0.0-20191219005021-4accf56bd2ce |
There was a problem hiding this comment.
Not used? Probably needs a go mod tidy (or you may not have checked something in?).
marten-seemann
pushed a commit
that referenced
this pull request
Mar 2, 2023
* feat(datatransfer): define revalidation interfaces * feat(message): add voucher results to message * feat(graphsyncimpl): add send voucher add send voucher function to data transfer, and create update options for send & receive * refactor(datatransfer): minor cleanups add utilties to channel state and extract integration tests * feat(message): add pause to message add a pause attribute message * feat(graphsyncimpl): switch to using messages instead of custom extension * feat(graphsyncimpl): convert to single roundtrip pull Setup pull requests to immediately send a GS request * refactor(datatransfer): refactor out transports Move all transport specific actions outside of the manage, into a transport layer than abstracts the underlying transport medium * feat(channels): add channel status tracking track channel status, add new events and statuses and move start/stop out of constructor * feat(receiver): refactor to use transport refactor receiver to use transport to handle responses * fix(impl): bug fixes and changes to work for sure with transport protocol * feat(channels): convert channel to cbor-gen Conver Channel to cbor-gen type * feat(channels): refactor to use fsm Refactor channels to use FSM, so we can track statuses and publish events automatically * feat(impl): implment pause/resume/cancel also add some voucher result processing. this includes a major refactor of the testing strategy for the implementation to be more of a unit test * feat(datatransfer): complete pause & resume Complete implementation of data transfer pause and resume * feat(datatransfer): support pause on first validate Modify voucher validation to support pauses on first validate * feat(datatransfer): add pause resume vouchers add the final step for a retrieval flow -- being able to stop and start via vouchers * feat(network): add connection tagging support connection tagging to protect connections from getting dropped * refactor(impl): split up files a bit more Split up functions into files so that they group better * fix(lint): fix lint & mod tidy * refactor(tests): pull out stubbed validators * fix(test): switch to table test for idempotene switch integration test to table test to make sure not to reuse constructs taht are already in use * fix(receiver): only resume transport when possible Only resume transport when it's possible for the transport to be resumed * fix(deps): update graphsync
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goals
Remove the concept of an IPLD Bridge. This bridge was created in early development of go-graphsync to mock IPLD when there was no working selector implementation. It has become largely useless, and a source of code bloat and an extra line in every setup of graphsync. Mocking can be done through the use of test ipld nodes that have specific selector behaviors.
Implementation
For discussion
Using the "real ipld" in all tests makes them not quite as "unit test"-ish -- but IPLD is not exactly an external dependency with side effects so much as a library with algorithms that get executed. As such it feels ok to use the real one to me. Morever, use of the testbridge obscured functioning and in at least one case revealed a test to not actually be verifying correct behavior.