homework19.1 #27
homework19.1 #27
Conversation
| def _prepare_params(self, params: dict) -> dict: | ||
| """Check if all required params are available.""" | ||
| if not isinstance(params, dict): | ||
| raise ValueError('Params must be a dictionary.') | ||
| if 'api_key' not in params: | ||
| raise ValueError("The 'api_key' parameter is required.") | ||
| return params |
There was a problem hiding this comment.
This looks redundant for this task - lets remove
| self.params = self._prepare_params(params) | ||
|
|
||
| @staticmethod | ||
| def _clean_url(url: str) -> str: |
There was a problem hiding this comment.
this looks redundant too - just use url from the beginnig without < and > signs
| try: | ||
| response = requests.get( | ||
| self.base_url, | ||
| params=self.params, | ||
| timeout=10, | ||
| ) | ||
| response.raise_for_status() | ||
| data = response.json() | ||
| return data.get('photos', []) | ||
| except (HTTPError, ConnectionError, Timeout) as err: | ||
| _log.error(f'Error fetching photos: {err}') | ||
| return [] |
There was a problem hiding this comment.
we have some logic for get request execution. This logic we use twice - > here and in photo download. So better to create helper function aka get_request and use it in both places
| failed_downloads += 1 | ||
| continue | ||
|
|
||
| filename = f'mars_photo{idx}.jpg' |
There was a problem hiding this comment.
This looks as naming template . we could use it as constant or class attribute and then fill with format() call
| with requests.get(photo_url, stream=True, timeout=10) as resp: | ||
| resp.raise_for_status() | ||
| with open(filename, 'wb') as fh: | ||
| for chunk in resp.iter_content(chunk_size=8192): |
There was a problem hiding this comment.
All hardcodes aka chunk_size, timeout etc better to move to init
| # End message | ||
| if failed_downloads: | ||
| _log.info(f'{failed_downloads} photo(s) could not be downloaded.') | ||
| else: | ||
| _log.info('All photos downloaded successfully!') |
There was a problem hiding this comment.
These logic should be hidden to main block below. Failued downloads could you could get as function call return value
vladspirin
added
the
need to improve
Cool task. I like to work with API.