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.

Sign up for GitHub

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

Jump to bottom

Add a context manager object and a method to call said object #357

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

Add a context manager object and a method to call said object #357

Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
8d1ae15
quick context manager prototype
Oct 20, 2020
529f0a4
adding some defaults
Oct 20, 2020
5bac851
default param added
Oct 20, 2020
4eeed1a
assignment, duh
Oct 20, 2020
4714ba4
debugging
Oct 20, 2020
021661d
dont delete feature flag data if exception was raise
Oct 20, 2020
2401f78
some quick refactoring to flush data when getting out of scope
Oct 20, 2020
00e4967
adding user default params
Oct 20, 2020
9c51dcc
Utilize linked list structure for scope of context manager
Oct 28, 2020
642cc20
Create watch wrapper method and rename FeatureFlags
Nov 4, 2020
ea80652
Revert renamte of FeatureFlags
Nov 4, 2020
83b77b5
Generalize FeatureFlags context manager
Nov 6, 2020
9e4c34a
Remove unnecessary re-assignment
Nov 9, 2020
5b63fc0
Clear out scopes after report_exc_info is called
Nov 9, 2020
0483523
Merge branch 'master' into context-manager-ld
Nov 10, 2020
ae692bf
Clear scope on exit only
Nov 11, 2020
fa6cf69
Change reference to scope to tag and force extra_data to be a dict
Nov 11, 2020
e7c0b5c
Attach the tag to the exception on exit
Nov 12, 2020
8d89888
Refactor tags check
Nov 17, 2020
9ca6df6
Implement specific feature flags interface
Nov 17, 2020
48eaa57
change context manager object to take in dict
Nov 17, 2020
e5fb916
Make feature flags stack thread safe
Nov 17, 2020
5bae76e
Use tags instead of feature flags
Nov 18, 2020
7378835
Add check for attr
Nov 18, 2020
69b035e
Model feature flags with tags
Nov 20, 2020
430fd29
Fix typo in comment
Nov 24, 2020
5eb2da2
Add test cases for the feature_flag context manager
Nov 25, 2020
00c89ec
Use [0][0] to index mocked object
Nov 25, 2020
25045f9
Remove feature_flag.data.order
Nov 25, 2020
bcece26
use self.tags rather than self.tag
Nov 30, 2020
1153529
Initialize _tags in thread_local on append and value
Dec 1, 2020
d3894db
Move flattening to _LocalTags and rename _tags
Dec 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion rollbar/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,7 +1648,10 @@ def pop(self):

@property
def value(self):
return self._registry.tags
if hasattr(self._registry, 'tags'):
return self._registry.tags

return []
Copy link
Author

Choose a reason for hiding this comment

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

why or when would be the case where _LocalTags is initialized, but does not have a tags attribute in self._registry?

removing this if condition, fails this test https://github.com/rollbar/pyrollbar/blob/master/rollbar/test/test_rollbar.py#L289

with this:

test_report_exception_with_same_exception_as_cause (rollbar.test.test_rollbar.RollbarTest) ... ERROR:rollbar:Exception while reporting exc_info to Rollbar. AttributeError("'thread._local' object has no attribute 'tags'",)
Traceback (most recent call last):
  File "/Users/ajtran/Development/tmp/test-launchdarkly/pyrollbar/rollbar/__init__.py", line 439, in report_exc_info
    return _report_exc_info(exc_info, request, extra_data, payload_data, level=level)
  File "/Users/ajtran/Development/tmp/test-launchdarkly/pyrollbar/rollbar/__init__.py", line 722, in _report_exc_info
    tags = _tags.value + getattr(exc_info[1], '_rollbar_tags', [])[::-1]
  File "/Users/ajtran/Development/tmp/test-launchdarkly/pyrollbar/rollbar/__init__.py", line 1652, in value
    return self._registry.tags
AttributeError: 'thread._local' object has no attribute 'tags'

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should try to get the root exception that is triggering that report and see at which moment that's happening

Copy link
Author

Choose a reason for hiding this comment

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

Ah. Ok ok. I think I see what is happening right now. For this test test_report_exception_with_same_exception_as_cause, we are running the report_exc_info inside a threading.Thread and when we do that, tags is not set in that thread._local, thus this runtime error when trying to call report_exc_info. I think I might know a way around this.

Copy link
Author

Choose a reason for hiding this comment

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

I've resolved this by initializing self._registry.tags in the append and value method of _LocalTags.



_tags = _LocalTags()
Expand Down