-
Notifications
You must be signed in to change notification settings - Fork 1
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
adding structured output option #10
Conversation
…g; see description and sample output added to README
lightbeam/send.py
Outdated
@@ -109,10 +156,27 @@ async def do_post(self, endpoint, file_name, data, client, line, hash): | |||
# warn about errors | |||
if response.status not in [ 200, 201 ]: | |||
message = str(response.status) + ": " + util.linearize(body) | |||
|
|||
# update run metadata... | |||
if "failed_statuses" not in self.metadata["resources"][endpoint].keys(): |
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.
Consider isolating the failed statuses dict as a variable to streamline updates.
failed_statuses_dict = self.metadata["resources"][endpoint].get("failed_statuses", {})
if response.status not in failed_statuses_dict:
failed_statuses_dict.update({response.status: {}})
if message not in failed_statuses_dict[response.status]:
failed_statuses_dict[response.status].update({message: {}})
if "files" not in failed_statuses_dict[response.status][message]:
failed_status_dict[response.status][message].update({"files": {}})
if file_name not in failed_statuses_dict[response.status][message]["files"]:
failed_statuses_dict[response.status][message]["files"].update({file_name: {}})
if "line_numbers" not in failed_statuses_dict[response.status][message]["files"][file_name]:
failed_statuses_dict[response.status][message]["files"][file_name].update({"line_numbers": []})
failed_statuses_dict[response.status][message]["files"][file_name]["line_numbers"].append(line)
Additionally, this is a very example of a defaultdict use-case, but it's multiple layers nested which might be difficult to set-up. If we were able to do so, it would convert this entire code-block to the following:
self.metadata["resources"][endpoint]["failed_statuses"][response.status][message]["files"][file_name]["line_numbers"].append(line)
I'm going to play around with this concept a little more.
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 see what you're saying, and this could be cleaner. One little bug in what you posted above, I think I still need to check for values in objects' .keys()
, since they're nested dicts. You have
if response.status not in failed_statuses_dict:
failed_statuses_dict.update({response.status: {}})
which I think still needs to be
if response.status not in failed_statuses_dict.keys():
failed_statuses_dict.update({response.status: {}})
(etc.)
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.
@jayckaiser I made another update along these lines in this commit and I tested it. If this is good enough for now, I can merge and we could consider refining later?
This PR implements the structured output portion of this issue. (A separate one will be coming to implement the exit code portion.) It adds the
--results-file
flag which allows you to specify an output to file to which earthmover will write a structured JSON run summary.Example JSON output can be found in the addition to the README in this PR.