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

[WIP] update celery context #1186

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

ryan-s
Copy link

@ryan-s ryan-s commented Jan 19, 2018

The current implementation of the celery client prevents the ability to pass context on a per process basis. We have found that being able to tag and filter based on the arguments or key word arguments is immensely helpful in tracking down where failures are in the system.

@ashwoods ashwoods self-requested a review January 19, 2018 14:30
@ashwoods
Copy link
Contributor

ashwoods commented Jan 19, 2018

@ryan-s Mind adding a small test? :) I forgot starting the comment with a thank you.

@ashwoods
Copy link
Contributor

ashwoods commented Feb 1, 2018

@ryan-s It's hard to see from the logs (a problem with xdist maybe), but this is still failing three tests:

_______________________________________________________________________ CeleryTestCase.test_context_args ________________________________________________________________________
Traceback (most recent call last):
  File "/Users/ashwoods/Documents/Code/sentry/raven-python/tests/contrib/test_celery.py", line 53, in test_context_args
    assert self.client.context
AssertionError: assert <Context: {}>
 +  where <Context: {}> = <raven.utils.testutils.InMemoryClient object at 0x10eac3e48>.context
 +    where <raven.utils.testutils.InMemoryClient object at 0x10eac3e48> = <tests.contrib.test_celery.CeleryTestCase testMethod=test_context_args>.client
______________________________________________________________________ CeleryTestCase.test_context_kwargs _______________________________________________________________________
Traceback (most recent call last):
  File "/Users/ashwoods/Documents/Code/sentry/raven-python/tests/contrib/test_celery.py", line 60, in test_context_kwargs
    assert self.client.context
AssertionError: assert <Context: {}>
 +  where <Context: {}> = <raven.utils.testutils.InMemoryClient object at 0x10e68d048>.context
 +    where <raven.utils.testutils.InMemoryClient object at 0x10e68d048> = <tests.contrib.test_celery.CeleryTestCase testMethod=test_context_kwargs>.client
__________________________________________________________________________ CeleryTestCase.test_simple ___________________________________________________________________________
Traceback (most recent call last):
  File "/Users/ashwoods/Documents/Code/sentry/raven-python/tests/contrib/test_celery.py", line 36, in test_simple
    assert event['culprit'] == 'dummy_task'
KeyError: 'culprit'

@ryan-s
Copy link
Author

ryan-s commented Feb 2, 2018

@ashwoods Ill be circling back to this next week. Getting the client to update the context in the logging with the mocks has been problematic. It's also becoming clear that Im conflating the logger and the celery handlers concepts, so I'm going to look to see if there is a more elegant way to handle this.

@ashwoods ashwoods changed the title update celery context [WIP] update celery context Feb 4, 2018
@ashwoods
Copy link
Contributor

ashwoods commented Feb 4, 2018

@ryan-s thx, let me know if there is anything I can do to help.

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.

2 participants