-
Notifications
You must be signed in to change notification settings - Fork 42
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
Handle empty reponses from Google APIs for listing requests #373
Conversation
if not releases: | ||
self.logger.info(Colors.YELLOW(f"No releases available for {app_identifier.app_id}")) | ||
else: | ||
printer = ResourcePrinter(json_output, self.echo) | ||
printer.print_resources(releases, should_print) | ||
|
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.
if not releases: | |
self.logger.info(Colors.YELLOW(f"No releases available for {app_identifier.app_id}")) | |
else: | |
printer = ResourcePrinter(json_output, self.echo) | |
printer.print_resources(releases, should_print) | |
if not releases: | |
self.logger.info(Colors.YELLOW(f"No releases available for {app_identifier.app_id}")) | |
return [] | |
printer = ResourcePrinter(json_output, self.echo) | |
printer.print_resources(releases, should_print) | |
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.
In expected scenarios this equivalent to current implementations. As with the comment above, if the releases
indeed comes out as None
(or any other falsy equivalent), then it is better if that unexpected value somehow surfaces.
@@ -65,7 +65,9 @@ def list( | |||
request = self._get_resources_page_request(page_request_args) | |||
response = self._execute_request(request) |
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.
The response might as well be ""
or even None
response = self._execute_request(request) | |
response = self._execute_request(request) or {} |
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.
For listing requests? If the response indeed is something other than what we expect now (a dictionary) then I find it to be better if the request handling fails and we'll become aware of the unexpected return type instead of blindly accepting all kinds of values.
Firebase releases API has started to return empty respones to listing requests. Handle those by checking if the requested resource key is present in the response object instead of failing unexpectedly with
KeyError
.Example traceback:
Updated actions:
firebase-app-distribution get-latest-build-version
firebase-app-distribution releases list