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

Add support for publishing events with em-synchrony #2

Merged
merged 3 commits into from
Jan 22, 2013

Conversation

alexkwolfe
Copy link
Contributor

This pull request adds support for asynchronous publishing using synchronous-style em-synchrony calls.

Specs are run using a separate rake task because requiring em-synchrony/em-http is destructive to em-http-request and therefore affects non-em-synchrony specs run in the same process.

@joshed-io
Copy link
Contributor

Thanks for the pull request, will be looking at this today.

@joshed-io
Copy link
Contributor

Will wait for the Fibers commit than I am happy to pull this in.

@alexkwolfe
Copy link
Contributor Author

I'm not sure why, but there's a problem running this under JRuby. I'll investigate and see what I can find out.

https://travis-ci.org/alexkwolfe/keen-gem/jobs/4291148

@joshed-io
Copy link
Contributor

That's ok - I'm not running the async/EM specs in jRuby (there's a JRUBY_VERSION defined check in client_spec.rb and api_spec.rb). TLS isn't implemented in jRuby EM so we're out of luck with SSL connections. So you can just add that same check around the synchrony specs.

@joshed-io
Copy link
Contributor

FWIW for anyone who stumbles on this - If you're trying to publish asynchronously from jRuby I'd suggest using the synchronous publish method wrapped in a Fiber or a Thread (or ideally a Thread or Fiber Pool). You'll still get the performance gains while avoiding jRuby+EM problems.

# asynchronous publishing without EM
Fiber.new {
  Keen.publish("sign_ups", name: "Bob")
}

joshed-io added a commit that referenced this pull request Jan 22, 2013
Add support for publishing events with em-synchrony
@joshed-io joshed-io merged commit ece8534 into keenlabs:master Jan 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants