Tokio migration #665
Tokio migration #665
Conversation
Fix apresolve WIP session [Core] More migration Playing with `ReadExact` and `WriteAll` Add some simple checks Take little steps
…respot into tokio_migration
Tokio migration
Implementing the tower_service::Service trait for a newly created ProxyTunnel struct, so it can be used as connector in hyper.
Proxy support in tokio migration
Merge dev into tokio migration
[Tokio migration] Add back hyper-proxy
|
@ashthespy Would you try to merge dev in? |
|
@Johannesd3 I'm away from a console for a bit longer, sorry! |
|
FWIW I've been running this branch without issue for about a week now. Given that the remaining todos are mostly think about/do we need items, and since #674 is now merged, now is probably as good a time as any to merge this into dev and release |
|
I certainly won't rebase 74 commits, we'll have to merge the changes of dev in. #687 contains dev until yesterday, but the latest PR by @roderickvd will be hard to merge ... EDIT: I merged also the latest PR. |
Librespot-connect uses hyper anyway, so no one needs to disable it only to reduce the number of dependencies. Furthermore, when using another backend, people will use --no-default-features and will forget to enable the apresolve feature again.
Some of the feature flags librespot uses are not really additive but rather mutual exclusive. A previous attempt to improve the situation had other drawbacks, so it's better to postpone a decision and restore the old behaviour.
fe37186 added the restriction that `Sink`s must be `Send`. It turned out later that this restrictions was unnecessary, and since some `Sink`s aren't `Send` yet, this restriction is lifted again. #601 refactored the `RodioSink` in order to make it `Send`. These changes are partly reverted in favour of the initial simpler design. Furthermore, there were some compile errors in the gstreamer backend which are hereby fixed.
The first test checks whether apresolve works. A second test tries to create a Spotify sessions with fake credentials and asserts that an error is returned.
Run the tests and add build for Windows.
Further progress on tokio migration
|
Well this PR is a monster, quite the achievement. Thanks for putting the work in. Are we happy to merge? |
|
There's no reason for it, but I'm still anxious. Maybe just wait 3 days, so everyone who is watching might test their own favourite features again after the lastest changes, but there are no outstanding or planned changes from my side. |
We're very close to finishing the migration to Tokio 1.0, and I think the result is worth seeing. Not only tokio but almost every other dependency was bumped to its latest version. In many places the code became simpler by using the async/await notation, and there were some general efforts of refactoring the code on this occasion.
Issues:
Pull requests:
Things that were removed and might or might not be added back:
Defaultimpl ofSessionConfig(was added again in Update examples #663)LinearMapdependency inlibrespot-metadata(replaced byHashMapin a6ed685)fn get_credentials(removed in 9253be7, see Move get_credentials function to the application #596)and
Think about feature flags (see So many features flags... #648, clean up imports & clippy lints #652)(restored old behaviour and postponed)Fixes #677, fixes #596, fixes #368