-
Notifications
You must be signed in to change notification settings - Fork 699
Improvements and tweaks #649
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
base: master
Are you sure you want to change the base?
Changes from all commits
199b7ee
0301526
76ef4ee
7d008b4
e596be4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,26 +29,29 @@ class NBAStatsResponse(http.NBAResponse): | |
| def __init__(self, *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
| self._endpoint = None | ||
| self._normalized_dict_cache = None | ||
|
|
||
| @staticmethod | ||
| def _build_rows(headers, row_set): | ||
| return [dict(zip(headers, raw_row, strict=False)) for raw_row in row_set] | ||
|
|
||
| def get_normalized_dict(self): | ||
| if self._normalized_dict_cache is not None: | ||
| return self._normalized_dict_cache | ||
|
|
||
| raw_data = self.get_dict() | ||
|
|
||
| data = {} | ||
|
|
||
| legacy_headers = ["resultSets", "resultSet"] | ||
| is_legacy = set(legacy_headers) & set(raw_data.keys()) | ||
| legacy_headers = {"resultSets", "resultSet"} | ||
| raw_keys = raw_data.keys() | ||
| is_legacy = bool(legacy_headers & raw_keys) | ||
|
|
||
| if is_legacy: | ||
| if "resultSets" in raw_data: | ||
| results = raw_data["resultSets"] | ||
| if "Meta" in results: | ||
| return results | ||
| else: | ||
| results = raw_data["resultSet"] | ||
| results = raw_data.get("resultSets") or raw_data.get("resultSet") | ||
| if results and "Meta" in results: | ||
| self._normalized_dict_cache = results | ||
| return results | ||
| if isinstance(results, dict): | ||
| results = [results] | ||
| for result in results: | ||
|
|
@@ -61,27 +64,31 @@ def get_normalized_dict(self): | |
| endpoint_parser = get_parser_for_endpoint(self._endpoint, raw_data) | ||
| for name, dataset in endpoint_parser.get_data_sets().items(): | ||
| data[name] = self._build_rows(dataset["headers"], dataset["data"]) | ||
| except (KeyError, ImportError): | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why remove the import error here? To be honest, I'm not sure why it's even catching it here in the first place
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the import of get_parser_for_endpoint fails, that's a critical issue that should raise an exception, not silently |
||
| except KeyError: | ||
| pass | ||
|
|
||
| self._normalized_dict_cache = data | ||
| return data | ||
|
|
||
| def get_normalized_json(self): | ||
| if self._normalized_dict_cache is not None: | ||
| return json.dumps(self._normalized_dict_cache) | ||
| return json.dumps(self.get_normalized_dict()) | ||
|
|
||
| def get_parameters(self): | ||
| if not self.valid_json() or "parameters" not in self.get_dict(): | ||
| raw = self.get_dict() if self.valid_json() else None | ||
| if raw is None or "parameters" not in raw: | ||
| return None | ||
|
|
||
| parameters = self.get_dict()["parameters"] | ||
| parameters = raw["parameters"] | ||
| if isinstance(parameters, dict): | ||
| return parameters | ||
|
|
||
| parameters = {} | ||
| for parameter in self.get_dict()["parameters"]: | ||
| result = {} | ||
| for parameter in parameters: | ||
| for key, value in parameter.items(): | ||
| parameters.update({key: value}) | ||
| return parameters | ||
| result[key] = value | ||
| return result | ||
|
|
||
| def get_headers_from_data_sets(self): | ||
| raw_dict = self.get_dict() | ||
|
|
||
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.
Why the additional breakout of the exception?
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 code now checks the HTTP status code first (any >= 400) raising an Exception with the status code, before
validating JSON. This ensures HTTP errors are properly caught.