libtorrent icon indicating copy to clipboard operation
libtorrent copied to clipboard

Python bindings: add session_params

Open AllSeeingEyeTolledEweSew opened this issue 4 years ago • 10 comments

It looks like session.save_state() / session.load_state() are deprecated in 2.0.

We should add the new session_params to the python bindings.

  • [X] There's no first-party binding for settings_pack, so session_params(settings_pack) isn't usable. session_params(default_settings()) should work, but doesn't match a type signature
  • [X] session_params.dht_state doesn't match a type signature
  • [x] session_params.ext_state doesn't match a type signature. it probably should be removed
  • [X] session_params.ip_filter accessor seems to be by-value, not by-reference, so session_params.ip_filter.add_rule(...) has no lasting effect
  • ~~session_params.ip_filter == session_params.ip_filter is false~~
  • [x] write_session_params(session_params(), flags=0) returns None, should return {}
  • ~~session_params() == session_params() is false~~
  • ~~dht_state() == dht_state() is false~~
  • [ ] dht_state.nids doesn't match a type signature
  • [ ] dht_state.nodes doesn't match a type signature
  • [ ] dht_state.nodes6 doesn't match a type signature
  • [x] session.session_state() needs to be bound

I agree. A related issue is the default plugins in python.

ut_metadata, ut_pex and smart_ban are all plugins that are added to the session by default. in C++ these are now controlled via the session_params constructor (which doesn't exist in python). But python can't really handle these plugins either, because they can't be implemented in python.

these plugins could be strings on the python sides. But I think there's really no good reason for these features to be implemented as plugins, they should just be merged into the main implementation and not be optional (or only be optional via settings). That way plugins remain out-of-scope for the python bindings.

arvidn avatar Apr 15 '21 08:04 arvidn

Sounds good to me.

how does this look? https://github.com/arvidn/libtorrent/pull/6164

arvidn avatar Apr 23 '21 09:04 arvidn

EDIT: moved to description

some of that is a addressed here: https://github.com/arvidn/libtorrent/pull/6204

arvidn avatar May 16 '21 07:05 arvidn

  • write_session_params(session_params(), flags=0) returns None, should return {}
  • session_params() == session_params() is false
  • dht_state() == dht_state() is false
  • dht_state.nids doesn't match a type signature
  • dht_state.nodes doesn't match a type signature
  • dht_state.nodes6 doesn't match a type signature

Before applying more fixes, would you mind approving #5990? I could submit PRs for these but it's far easier to do that with test suite improvements.

Also I noticed that session_params().settings != default_settings(). Should it be?

I think ext_state should probably just not be bound in python, continuing the current principle that extensions should be opaque to python.

I guess we should also put off implementing __eq__ for ip_filter, dht_state and session_params, since operator== isn't implemented in C++ on these.

I don't know how to make dht_state.nids, dht_state.nodes, and dht_state.nodes6 work. It looks like the converters are set up correctly, but they fail at runtime.