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

JRuby support #19

Closed
sobrinho opened this issue Jul 17, 2012 · 10 comments
Closed

JRuby support #19

sobrinho opened this issue Jul 17, 2012 · 10 comments

Comments

@sobrinho
Copy link
Contributor

yajl-ruby does not compile in jruby.

On a mixed environment like ruby, python, java, etc I think XML works better.

If JSON is the best choice for raven-ruby, we could use something like multi_json and let developer decide which json library he want.

What do you think?

I can make a pull request if you accept the change to multi_json :)

@coderanger
Copy link
Contributor

JSON is required as that is what the server speaks (as is the case with 99% of APIs these days). As for multi_json, I've got nothing against a patch like that, as long as on normal Ruby it continues to install a faster library by default. I'm not super familiar with JRuby, so I'm not sure if you can make conditional dependencies somehow.

@sobrinho
Copy link
Contributor Author

@coderanger I made the commit but since we don't have specs covering this piece of raven-ruby, I will run my fork on my staging environment to check if it's ok.

See https://github.com/sobrinho/raven-ruby/commit/f80196498cfdaa82e79f02340b3365a7b034fb9d

@coderanger
Copy link
Contributor

This will mean that if the user is on MRI and they don't manually install yajl or oj, they are stuck with the slower standard library one. I'm not a fan of that outcome.

@sobrinho
Copy link
Contributor Author

@coderanger you should think about how many times and how many important is the time spent on JSON encoding time.

If it took 3sec to encode, could be ok if the application raises one exception per day.

If the application raises 1,000 exceptions per minute, could be a problem.

I think the mostly part of application raises few exceptions instead of a lot of exceptions.

@coderanger
Copy link
Contributor

Sentry can also be used for straight logging, not just exceptions.

@sobrinho
Copy link
Contributor Author

Using multi_json make the developer decide which json implementation is better for each application.

Some applications needs a fast json implementation but some don't, we can't guess that.

Logging things to sentry does not mean the spent time on json encoding is expensive, it just means you are logging things.

In highly accessed applications could be a problem, in smaller applications could not.

If you need a fast json implementation, you just need to add it to gemfile and multi_json will take care about the rest:

gem 'sentry-raven'
gem 'yajl-ruby'

Using multi_json means we are not assuming things about application environment.

If you find it necessary, we could write a read me section telling about multi json :)

@coderanger
Copy link
Contributor

You make a compelling case, but the other side of the coin is good defaults. It just seems like the current state of Ruby packaging requires conservative defaults, rather than fast ones.

@sobrinho
Copy link
Contributor Author

@coderanger thanks :)

see #20 ;)

@lsb
Copy link
Contributor

lsb commented Aug 22, 2013

Just out of curiosity, was the JSON library the only thing that was breaking in JRuby?

@sobrinho
Copy link
Contributor Author

@lsb in that time, yeah.

Did you find another issues?

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

No branches or pull requests

3 participants