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

Error handling in downloadThread #227

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Error handling in downloadThread #227

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1fe835
fix: error handling in downloadThread
Feb 8, 2024
2cb81bb
fix: proper log error for the exception
Feb 10, 2024
b993098
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 10, 2024
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
6 changes: 3 additions & 3 deletions osm_fieldwork/OdkCentral.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ def downloadThread(project_id: int, xforms: list, odk_credentials: dict, filters
form = OdkForm(odk_credentials["url"], odk_credentials["user"], odk_credentials["passwd"])
# submissions = form.getSubmissions(project_id, task, 0, False, True)
subs = form.listSubmissions(project_id, task, filters)
if type(subs) == dict:
if subs == {}:
Copy link
Member

@spwoodcock spwoodcock Feb 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

if subs == {}, then subs["message"] will not exist, so the log.error will fail

Copy link
Member

Choose a reason for hiding this comment

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

What format is the error JSON if getting the submission fails?

You could probably do something like:

if isinstance(subs, dict) and (error_code := subs.get("code", False)):
    log.error(
        f"Failed to get submissions for project ({project_id}) task ({task})"
        f" with error code {error_code}: {subs.get('message')}"
    )
    continue
if len(subs.get("value"), []) > 0:
    data += subs["value"]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes you are right.
our listSubmissions() looks like this:

url = f"{self.base}projects/{projectId}/forms/{xform}.svc/Submissions"
result = self.session.get(url, params=filters, verify=self.verify)
if result.ok:
    self.submissions = result.json()
    return self.submissions

return {}

if result is not ok that means status code returned by the server is not in the range of 200-299,
empty dict is returned. so there wont be message and status code.
is this log okay?

if subs == {}:
    log.error(f"Failed to get submissions for project ({project_id}) task ({task})")
    continue

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't give the reason for failure like the code above, but yes it's fine too

log.error(f"{subs['message']}, {subs['code']} ")
continue
# log.debug(f"There are {len(subs)} submissions for {task}")
if len(subs) > 0:
data += subs
if len(subs["value"]) > 0:
data += subs["value"]
# log.debug(f"There are {len(xforms)} Xforms, and {len(submissions)} submissions total")
timer.stop()
return data
Expand Down
Loading