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

[WIP] Replace tokio-core with tokio #317

Closed
wants to merge 8 commits into from

Conversation

GyrosOfWar
Copy link

Hi!

This is a pretty big change, but I feel like it improves usability a lot (not having to pass handles around etc.). One thing that's still missing is the ability to pass in your own reqwest::async::Client when building an async elastic client, which now might make more sense to have as a builder method.

@GyrosOfWar GyrosOfWar changed the title Replace tokio-core with tokio [WIP] Replace tokio-core with tokio Jan 4, 2019
@KodrAus
Copy link
Member

KodrAus commented Jan 6, 2019

Thanks for taking this on @GyrosOfWar!

There's a substantial set of breaking changes in the vNext branch, but once we've got this working on master I'll go ahead and rebase onto vNext.

It looks like our integration tests (they live here) are failing to run. If you've got docker available you should be able to run these integration tests yourself locally:

cd tests/run
cargo run

Please reach out if you've any questions at all!

@GyrosOfWar
Copy link
Author

I'll probably look into this over the weekend. Do you have any idea about why the bench build fails? (https://travis-ci.org/elastic-rs/elastic/jobs/475275138) I can compile the json_str crate by itself just fine, it's just when I try to compile the benches project that it fails.

@KodrAus
Copy link
Member

KodrAus commented Jan 10, 2019

Ah the benches are in a barely useful state right now and have suffered from bitrot, so I think we should get rid of them at this stage. Any new round of performance improvements can work with new benchmarks.

If you remove the bench environment that should be enough to unblock us.

@KodrAus
Copy link
Member

KodrAus commented Jan 26, 2019

Hi @GyrosOfWar. I've just gone and merged in about 2 years of work in progress from the vNext branch onto master. Rebasing this work onto master will probably be very tedious, but I think we should update these old packages before releasing the next version. If you're not keen to deal with that then I'm happy to grab your branch and carry it forward.

Thanks again for working on this!

@KodrAus KodrAus mentioned this pull request Jan 26, 2019
@KodrAus
Copy link
Member

KodrAus commented Jan 27, 2019

Alrighty, I've rolled this up into #320 so we now use tokio instead of tokio-core on master.

Thanks again for taking the time to do this!

@KodrAus KodrAus closed this Jan 27, 2019
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.

None yet

2 participants