-
Notifications
You must be signed in to change notification settings - Fork 134
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
Fix unbound local error exception #2428
Conversation
Fixes UnboundLocalError local variable 'event_source' referenced before assignment exception Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
Let's have this component disabled by default too |
onadata/libs/utils/analytics.py
Outdated
@@ -136,18 +136,16 @@ def get_event_label(self) -> str: | |||
def get_request_origin(self, request, tracking_properties): | |||
"""Returns the request origin""" | |||
if isinstance(self.tracked_obj, Instance): | |||
event_source = "" # Initialize event_source variable | |||
web_user_agents = ["Chrome", "Mozilla", "Safari", "PostmanRuntime"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Postman is used to submit Enketo records ??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
onadata/libs/utils/analytics.py
Outdated
@@ -136,18 +136,18 @@ def get_event_label(self) -> str: | |||
def get_request_origin(self, request, tracking_properties): | |||
"""Returns the request origin""" | |||
if isinstance(self.tracked_obj, Instance): | |||
event_source = "" # Initialize event_source variable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this line to above line 138 and remove the else clause on line 150?
event_source = "" # Initialize event_source variable
if isinstance(self.tracked_obj, Instance):
event_source = "Submission collected from Web"
browser_user_agents = ["Chrome", "Mozilla", "Safari"]
try:
user_agent = request.META["HTTP_USER_AGENT"]
except KeyError:
pass
if "Android" in user_agent:
event_source = "Submission collected from ODK COLLECT"
elif any(ua in user_agent for ua in browser_user_agents):
event_source = "Submission collected from Enketo"
tracking_properties.update({"from": event_source})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
33ea555
to
9e50282
Compare
onadata/libs/utils/analytics.py
Outdated
event_source = "" | ||
pass | ||
|
||
if "Android" in user_agent: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe user_agent
won't be defined in scenarios where the KeyError is raised
Signed-off-by: Kipchirchir Sigei <[email protected]>
9e50282
to
8869037
Compare
Changes / Features implemented
UnboundLocalError
exception thrown for uninitializedevent_source
varSteps taken to verify this change does what is intended
Side effects of implementing this change
Before submitting this PR for review, please make sure you have:
Closes #2424 and #2427