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

remove bitswap unwant #5308

Merged
merged 1 commit into from
Aug 28, 2018
Merged

remove bitswap unwant #5308

merged 1 commit into from
Aug 28, 2018

Conversation

Stebalien
Copy link
Member

This command messes with internal state and doesn't even work at the moment. If
you don't want a block, you should cancel the request that's trying to fetch it.

An alternative is to replace this command with a dummy command that prints an
error but we'll break user code either way and I'd rather delete code.

fixes #5295

@Stebalien Stebalien requested a review from Kubuxu as a code owner July 29, 2018 18:44
@ghost ghost assigned Stebalien Jul 29, 2018
@ghost ghost added the status/in-progress In progress label Jul 29, 2018
@Stebalien Stebalien added need/review Needs a review and removed status/in-progress In progress labels Jul 29, 2018
@ghost ghost added the status/in-progress In progress label Jul 29, 2018
@ivan386
Copy link
Contributor

ivan386 commented Jul 30, 2018

It was way to send message that you have block to connected peer that don't send you want message yet.

@Stebalien
Copy link
Member Author

It was way to send message that you have block to connected peer that don't send you want message yet.

I'm not sure if I understand what you're saying but this command attempts to cancel a want. That is, it removes a block from a wantlist, even if some other part of the system is requesting it. I'm proposing that we remove it because it destructively interferes with other commands and isn't the correct way to stop asking for a block.

@ghost
Copy link

ghost commented Jul 30, 2018

cc @alanshaw

@Stebalien
Copy link
Member Author

Stebalien commented Aug 16, 2018

@diasdavid this command really isn't the correct way to solve any problem and it interferes with other commands. 100% of the time, the user should cancel the command that is causing us to want something.

Would you object to removing this?

Copy link
Member

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

yes please

@daviddias
Copy link
Member

No blockers from me on removing it from the Public API. @Stebalien can you please (🙏🏽) give a pass through the specs while you issue this update?

//cc @alanshaw, @achingbrain & @hugomrdias

@Stebalien
Copy link
Member Author

@diasdavid done: ipfs/specs#188

@alanshaw
Copy link
Member

alanshaw commented Aug 23, 2018

@Stebalien what's your release plan for this? go-ipfs 0.5?

We can deprecate this now in js-ipfs-api (print a message to the console when used) and also in js-ipfs. There's an upcoming 0.32 js-ipfs release so it would be good to get the deprecation message in there and then remove it in the next release.

@Stebalien
Copy link
Member Author

what's your release plan for this

Just a normal release? We don't really follow semver (yet).

I really wasn't planning on having a deprecation process given that this doesn't currently work (in go at least) and nobody should be using it anyways. The user will just get a "command not found error".

I could also just switch it to a dummy command that prints an error.

@alanshaw
Copy link
Member

Gotcha, lets just remove it then!

This command messes with internal state and doesn't even work at the moment. If
you don't want a block, you should cancel the request that's trying to fetch it.

fixes #5295

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien Stebalien added RFM and removed status/in-progress In progress need/review Needs a review labels Aug 28, 2018
@Stebalien Stebalien merged commit 5c309cc into master Aug 28, 2018
@daviddias daviddias deleted the fix/5295 branch August 29, 2018 17:40
ntninja added a commit to ntninja/py-ipfs-api-client that referenced this pull request Jan 27, 2019
ntninja added a commit to ntninja/py-ipfs-api-client that referenced this pull request Jan 27, 2019
ntninja added a commit to ntninja/py-ipfs-api-client that referenced this pull request Jan 28, 2019
ntninja added a commit to ntninja/py-ipfs-api-client that referenced this pull request Jan 29, 2019
ntninja added a commit to ipfs-shipyard/py-ipfs-http-client that referenced this pull request Jan 29, 2019
ntninja added a commit to ipfs-shipyard/py-ipfs-http-client that referenced this pull request May 13, 2019
gersonkevin23 added a commit to gersonkevin23/py-ipfs-http-client that referenced this pull request Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bitswap unwant
5 participants