Conversation
- refactor get_parameters update {k:v} anti-pattern
- improve find_player_by_id with single dict get() call
- find_players search speedup
…mmit - status check in raise_exception_on_error raises on HTTP 4xx/5xx before checking json validity (previously a 429 or 500 would silently pass)
…ecks) - added strip_accents for find_teams - ImportError should not be silenced as it could indicate a broken module (parser with syntax error)
| 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): |
There was a problem hiding this comment.
Why remove the import error here? To be honest, I'm not sure why it's even catching it here in the first place
There was a problem hiding this comment.
If the import of get_parser_for_endpoint fails, that's a critical issue that should raise an exception, not silently
continue with an empty result
|
No major complaints from my initial pass. I need to do a deeper dive here and then push this one through. All the changes you suggested make sense |
| if raise_exception_on_error and not data.valid_json(): | ||
| raise Exception("InvalidResponse: Response is not in a valid JSON format.") | ||
| if raise_exception_on_error: | ||
| if status_code is not None and status_code >= 400: |
There was a problem hiding this comment.
Why the additional breakout of the exception?
There was a problem hiding this comment.
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.
2 participants
I have analyzed my demo app along with external libraries used. It is using nba_api lib to keep track on recent games and player stats. Probably it would be good to make a revision on the fixes provided in this PR, especially the one reducing redundant json.loads by implementing dict_cache. Notable is reducing get_parameters() calls of get_dict() from 3 to 1 and find_players() compiles re only one per search now.
2nd batch of optimizations: