Skip to content
This repository was archived by the owner on Oct 23, 2023. It is now read-only.

Conversation

@char0n
Copy link

@char0n char0n commented May 29, 2013

I have implemented gearman support for raven sentry client. For more information what gearman is, and what is does please refer to http://gearman.org/.

This pull request includes:

  • gearman support
  • tests
  • documentation

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 2a78870 on CodeScaleInc:master into d0bd88f on getsentry:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0%) when pulling 952bc87 on CodeScaleInc:master into d0bd88f on getsentry:master.

@xordoquy
Copy link
Contributor

Hi,

Thanks for your pull request.
I just had a small look at it.
Is there any reason why you need to introduce new django commands with the raven client ?
Will the regular Django client still work if one doesn't have gearman installed ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Keeping the comma avoid to see that line in the diffs if we had another item in the list after it.

@char0n
Copy link
Author

char0n commented Jun 21, 2013

Is there any reason why you need to introduce new django commands with the raven client ?

The reason is, that gearman workers are implemented via https://github.com/CodeScaleInc/django-gearman-commands app. Basically it is classic django command, that runs in infinite loop and process jobs from gearman daemon in separate thread. For more information how this works, please visit the github repo for django-gearman-commands.

Will the regular Django client still work if one doesn't have gearman installed ?

Yes it will. This is my setting from django project.

SENTRY_CLIENT = 'raven.contrib.django.gearman.GearmanClient'
RAVEN_CONFIG = {
'dsn': 'secret_dns_string,
'timeout': 3
}

Only when using GearmanClient in your settings, gearman daemon is required. When your are using GearmanClient and gearman daemon is not installed ConnectionError is raised. When using standard DjangoClient, gearman daemon is no required at all.

I hope this answered your questions.

@dcramer
Copy link
Member

dcramer commented Jun 21, 2013

IIRC this will force raven_gearman.py to be imported as part of Django's cycle, which would then throw an ImportError for anyone not using the gearman controls

@char0n
Copy link
Author

char0n commented Jun 21, 2013

I see your point now. I can make a little modification in raven_gearman.py then:

from future import absolute_import
from django.conf import settings

if gettattr('RAVEN_CONFIG', settings) == 'raven.contrib.django.gearman.GearmanClient':
from raven.contrib.gearman.worker import Command
else:
# Create dummy command that does nothing

Will this be satisfactory ?

@xordoquy
Copy link
Contributor

It looks a bit tricky to me.
Can't there be a django_gearman or simply gearman contrib module -ie out of the django one- which will be used if gearman is used and keep things as they already are with django ?

@dcramer
Copy link
Member

dcramer commented Jun 21, 2013

Ya ideally something like contrib.django.raven.gearman (or whatever namespace makes sense) that you can drop in your INSTALLED_APPS

@char0n
Copy link
Author

char0n commented Jun 21, 2013

Ok sounds good. I can make contrib.django.gearman as an django app that can be dropped easily in INSTALLED_APPS.

@xordoquy
Copy link
Contributor

xordoquy commented Aug 9, 2013

You need to add

from __future__ import absolute_import

on top of your files so that the builds can be run by travis

@xordoquy
Copy link
Contributor

ok, I'm going to close this. Note that raven will undergo this year a movement to split specific framework appart from the core.

@xordoquy xordoquy closed this Jan 21, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants