Option to disable BEP19 web seeds

[dcposch]

• For completeness. We have options to enable/disable WebRTC, the regular BitTorrent tracker client, and the DHT client. This lets us also enable/disable web seeds.
• Useful for WebTorrent Desktop integration testing. See electron/electron#7212

[DiegoRBaquero]

LGTM. On another topic, should we add opts.maxWebConns to the client? It's in the torrent.

[feross]

In the future, please don't change the package.json version in a PR. That's best done at release time with the npm version command (i.e. npm version patch, npm version minor, npm version major).

[feross]

If you're curious about the exact commands I use to do a release, they are here: https://github.com/feross/prezto/blob/master/runcoms/zshrc#L31-L33

[feross]

Sorry -- I see now that you did make a separate commit for the version bump. Still, it's not good to encourage this in PRs. When random people send you a PR and increment the version, it makes it harder to merge that PR.

[dcposch]

I did use npm version minor

I bumped the version because I need it for the desktop app: webtorrent/webtorrent-desktop#913

[feross]

Looks like v0.96.0 is missing. I'll add it.

[feross]

PR sent: #915

[dcposch]

Left comments re some typos but otherwise looks good

[feross]

This would have been better as singular, webSeed IMO since we didn't name the other option trackers. No worries, I'll change it along with the other breaking API changes in v1.0.

[dcposch]

I wanted to match webConns. The other options, tracker and dht, both take an object, this one just takes true or false.

I think it's fine but have no strong opinion, if you want to change it in 1.0 that's cool too

[feross]

Won't this just add flakiness to the test suite? We don't do this in any of the other tests. What if it takes 600ms to complete on Travis CI servers?

[dcposch]

It completes very fast (~10ms) on my Macbook.

I added the timeout to match the timeout below. How do you verify that webSeeds: false is working? I thought: make sure that the webSeed download always completes in under X time, then set webSeeds: false and wait that same amount of time and verify there are no requests

[feross]

I'm not sure what this comment means. Since the webSeeds option was set to false, no download progress will be made, right? This will just timeout after 500ms and move on to the cleanup code below.

That's fine, I think waiting 500ms is a reasonable way to ensure that no http requests get sent to the server, but the comment should be changed to be clearer.

[feross]

Also, another test we can add here is:

var torrent = client.add(parsedTorrent, {store: MemoryChunkStore})

torrent.on('ready', function () {
  t.equal(torrent.urlList.length, 0) // ensure no web seeds were added
})

[dcposch]

"The test above" refers to the test that has webSeeds enabled. There are two tests in the file, the first ensures that web seeds work, the second ensures that webSeeds: false works

[dcposch]

I don't think that extra test works. It doesn't prevent urlList from being populated (just as setting tracker: false won't make the announce list empty, right?). It just means that none of those URLs will be contacted.

[feross]

Overall, looks great to me! Sorry for being so behind on reviews. I think it's great that you merged this, but please address the inline comments and send another PR to fix things up!

[lock]

This thread has been automatically locked because it has not had recent activity. To discuss futher, please open a new issue.