Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to go-data-transfer v2 #103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

hannahhoward
Copy link
Contributor

Goals

Well, this was going to be an update to go-data-transfer v2... but then it turns out filclient is way behind on Libp2p so, that got added, and then it's way behind on Lotus, so that got added... plus then also it's dependent on go-fil-markets being up to date for data-transfer-v2 so I updated that, but then also, apparently, it uses Lotus's itest framework...

so currently I guess this is blocked pending getting freakin lotus up to date with go-data-transfer v2?

Anyway, @rvagg good time to review I guess.

@hannahhoward
Copy link
Contributor Author

eek this should be a draft anyway don't merge.

@@ -1322,7 +1318,7 @@ func (fc *FilClient) CheckChainDeal(ctx context.Context, dealid abi.DealID) (boo
func (fc *FilClient) CheckOngoingTransfer(ctx context.Context, miner address.Address, st *ChannelState) (outerr error) {
defer func() {
// TODO: this is only here because for some reason restarting a data transfer can just panic
// https://github.com/filecoin-project/go-data-transfer/issues/150
// https://github.com/filecoin-project/go-data-transfer/v2/issues/150
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// https://github.com/filecoin-project/go-data-transfer/v2/issues/150
// https://github.com/filecoin-project/go-data-transfer/issues/150

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the https url is unchanged. yay go modules being weird.

Copy link
Collaborator

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sad, that it's so minimal and close.

there's also a go-ipld-prime@v0.19.0 as of today that you could add in here while you're upgrading it

@rvagg
Copy link
Collaborator

rvagg commented Oct 14, 2022

oh, you need to upgrade go.mod to 1.18 too I think, generics in Lotus are making it fail to build as well

@neelvirdy
Copy link
Contributor

@hannahhoward @rvagg is this unblocked now? i would like to get this through to unblock estuary using the FastRetrieval / KeepUnsealedCopy flag when proposing deals through filclient

@rvagg
Copy link
Collaborator

rvagg commented Jan 24, 2023

@neelvirdy would you mind taking this over and seeing if you can get the dependencies updated? I suspect it might still be blocked by Lotus, I haven't followed the status of libp2p dependencies over there.

We got unblocked by removing lotus dependency in Lassie and doing basically this same PR over there: filecoin-project/lassie#15 but that's not as easy an option here because of the need for lotus dependencies for the rest of filclient's APIs that we don't implement in Lassie.

@neelvirdy
Copy link
Contributor

@neelvirdy would you mind taking this over and seeing if you can get the dependencies updated? I suspect it might still be blocked by Lotus, I haven't followed the status of libp2p dependencies over there.

We got unblocked by removing lotus dependency in Lassie and doing basically this same PR over there: filecoin-project/lassie#15 but that's not as easy an option here because of the need for lotus dependencies for the rest of filclient's APIs that we don't implement in Lassie.

@rvagg i updated dependencies to the point of getting boost to latest, which was actually all i needed to do #126. i've setup dependabot on this repo to keep it from falling too far behind and i will monitor the ones that get blocked. i dont have a direct need for the go-data-transfer/v2 migration itself as of now, so will hold off on that part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants