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

Switch from AnyEvent to IO::Async #1912

Merged
merged 4 commits into from Jun 14, 2017
Merged

Switch from AnyEvent to IO::Async #1912

merged 4 commits into from Jun 14, 2017

Conversation

haarg
Copy link
Member

@haarg haarg commented Jun 11, 2017

We use AnyEvent to run multiple HTTP requests simultaneously. This converts those to using Net::Async::HTTP. All of the API model methods have been changed to consistently return futures that can be fetched with ->get, which should make it easier to clean up how we are using concurrent requests.

Additionally includes a rewrite of the site map generation to something that should actually work, and some other prereq cleanups.

@haarg haarg force-pushed the haarg/io-async branch 2 times, most recently from 5878e7c to bf533c5 Compare June 11, 2017 19:59
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.3%) to 76.471% when pulling bf533c5 on haarg/io-async into 5cce45b on master.

@haarg
Copy link
Member Author

haarg commented Jun 12, 2017

I pulled some changes from this PR out into #1915 because they are simpler and don't rely on this. Will rebase this once that PR is merged.

@mickeyn
Copy link
Contributor

mickeyn commented Jun 12, 2017

@haarg merged

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 76.573% when pulling f92308c on haarg/io-async into 7dd8420 on master.

@mickeyn mickeyn requested review from mickeyn and removed request for mickeyn June 13, 2017 13:26
Copy link
Contributor

@mickeyn mickeyn left a comment

Choose a reason for hiding this comment

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

LGTM (though not sure if the sitemap change belongs in the same PR... but that's ok)

@haarg
Copy link
Member Author

haarg commented Jun 14, 2017

The sitemap changes could maybe go in a separate PR, but to handle it sensibly it would have to depend on this one, and require an additional cpanfile.snapshot regeneration. The existing sitemap code doesn't even work properly, and we aren't running it on the servers.

@mickeyn
Copy link
Contributor

mickeyn commented Jun 14, 2017

it's OK with me (i'm known to combine several issues in single PRs :))

@oalders oalders merged commit bb08de5 into master Jun 14, 2017
@oalders oalders deleted the haarg/io-async branch June 14, 2017 14:03
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

4 participants