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

Conversation

@gthb
Copy link
Contributor

@gthb gthb commented Nov 25, 2013

Support authentication setups where the user model does not derive from
Django's AbstractBaseUser or User. Instead accept duck typing and just
abort capturing the user if any of the required attributes are missing.

Support authentication setups where the user model does not derive from
Django's AbstractBaseUser or User. Instead accept duck typing and just
abort capturing the user if any of the required attributes are missing.
@dcramer
Copy link
Member

dcramer commented Nov 30, 2013

I'm a bit scared of this. What if we just document/ensure its easy to inject your own extensions to Client which would let you inject your own user information.

I think we explicitly added this check for a reason in the past, and try/except has bitten us before (AttributeError is also a bit too wide ranged).

@gthb
Copy link
Contributor Author

gthb commented Dec 3, 2013

Scared of what, exactly? (No argumentative tone intended, I just want to dig into this. : ) )

No reason was given for the check when it was introduced (in 039dbfa).

A custom extension would make sense if the information to be conveyed were different in some way, but in our case the model objects are identical in interface and information content (just queried/mapped with SQLAlchemy; same underlying table, same attributes). So supporting duck typing seems reasonable. Django itself and all third-party apps are fine with this; just not Raven.

@xordoquy
Copy link
Contributor

xordoquy commented Dec 3, 2013

@dcramer is that part of the deserialization in sentry ?

@dcramer
Copy link
Member

dcramer commented Dec 3, 2013

If GH-389 looks good, that would be our generic solution to solving this. I implemented the matching API in the PHP client last night, and I added notes about it to the client spec.

@gthb can you confirm that'd handle your case?

@gthb
Copy link
Contributor Author

gthb commented Dec 9, 2013

Sorry about the late reply. It's not clear how this new API is intended to be used, in a Django integration. Current Django config docs have us doing something like:

INSTALLED_APPS += ('raven.contrib.django.raven_compat',)
RAVEN_CONFIG = {
    'dsn': 'http://public:secret@example.com/1',
    # ...
}

but that doesn't suggest any obvious place to use this new API, as illustrated by this docstring:

        >>> def view_handler(view_func, *args, **kwargs):
        >>>     client.context.merge(tags={'key': 'value'})
        >>>     try:
        >>>         return view_func(*args, **kwargs)
        >>>     finally:
        >>>         client.context.clear()

Where would this view_handler be? Do we set up an additional middleware to wrap that view_handler around views? (So view_handler is really process_request in a middleware class?) Do we obtain that client using from raven.contrib.django.raven_compat.models import client?

@dcramer
Copy link
Member

dcramer commented Dec 9, 2013

Middleware is definitely the ideal way to solve this (and I think we'll eventually move that direction):

def process_request(self, request):
    client.user_context({'email': request.user.email})

And yeah, you're thinking on client is correct.

Does that make sense?

@gthb
Copy link
Contributor Author

gthb commented Dec 9, 2013

Sure, that will probably work; will try out later tonight or tomorrow.

@dcramer
Copy link
Member

dcramer commented Dec 10, 2013

@gthb feel free to reopen if you think we still need some kind of additional native support

@dcramer dcramer closed this Dec 10, 2013
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.

3 participants