Add precommit #9
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes across the project enhance code readability and consistency by standardizing string delimiters from single quotes to double quotes. A new exception class improves error handling in camera operations, while the introduction of a configuration file for pre-commit hooks ensures better code quality through automated checks. Overall, these updates streamline the code, maintain functionality, and make it easier for developers to work collaboratively. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Camera
participant ErrorHandler
User->>Camera: Request Snapshot
Camera-->>User: Provide Snapshot
User->>Camera: Request Reboot
Camera-->>ErrorHandler: Handle Reboot Error
ErrorHandler-->>User: Notify Error
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- .pre-commit-config.yaml (1 hunks)
- setup.py (1 hunks)
- tests/test_camera.py (2 hunks)
- tests/test_cli.py (1 hunks)
- tests/test_client.py (1 hunks)
- tests/test_store.py (1 hunks)
- uvcclient/camera.py (1 hunks)
- uvcclient/main.py (3 hunks)
- uvcclient/nvr.py (7 hunks)
- uvcclient/store.py (1 hunks)
Files skipped from review due to trivial changes (6)
- setup.py
- tests/test_camera.py
- tests/test_cli.py
- tests/test_client.py
- tests/test_store.py
- uvcclient/main.py
Additional context used
Ruff
uvcclient/store.py
30-30: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
39-39: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
uvcclient/camera.py
55-55: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
57-57: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
uvcclient/nvr.py
101-101: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
103-103: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
109-109: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
111-111: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
209-212: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Additional comments not posted (36)
.pre-commit-config.yaml (4)
1-4: LGTM!The initial comments provide useful links, and the exclude settings are appropriate.
6-10: LGTM!The CI settings are appropriate and provide useful configurations for pre-commit CI.
11-16: LGTM!The commitizen hook configuration is appropriate and ensures commit message consistency.
17-34: LGTM!The pre-commit hooks configuration is comprehensive and ensures code quality.
uvcclient/store.py (3)
17-17: LGTM!The string delimiter changes in the
__init__method improve consistency.
42-42: LGTM!The string delimiter changes in the
get_camera_passwordsmethod improve consistency.
48-50: LGTM!The string delimiter changes in the
set_camera_passwordmethod improve consistency.uvcclient/camera.py (11)
46-46: LGTM!The string delimiter changes in the
__init__method improve consistency.
60-87: LGTM!The string delimiter changes in the
loginmethod improve consistency.
90-96: LGTM!The string delimiter changes in the
_cfgwritemethod improve consistency.
100-100: LGTM!The string delimiter changes in the
set_ledmethod improve consistency.
104-104: LGTM!The string delimiter changes in the
snapshot_urlproperty improve consistency.
108-108: LGTM!The string delimiter changes in the
reboot_urlproperty improve consistency.
112-112: LGTM!The string delimiter changes in the
status_urlproperty improve consistency.
115-120: LGTM!The string delimiter changes in the
get_snapshotmethod improve consistency.
124-129: LGTM!The string delimiter changes in the
rebootmethod improve consistency.
132-137: LGTM!The string delimiter changes in the
get_statusmethod improve consistency.
147-156: LGTM!The string delimiter changes in the
loginmethod improve consistency.uvcclient/nvr.py (18)
49-50: LGTM!The
CameraConnectionErrorclass is a simple and appropriate addition for handling specific camera connection errors.
58-69: LGTM!The constructor's string formatting changes improve consistency and readability. The logic remains unchanged.
Line range hint
73-81:
LGTM!The
server_versionproperty's string formatting changes improve consistency. The logic remains unchanged.
85-87: LGTM!The
camera_identifierproperty's logic remains unchanged and is appropriate.
Line range hint
91-93:
LGTM!The
_get_http_connectionmethod's logic remains unchanged and is appropriate.
113-145: LGTM!The
_uvc_request_safemethod's string formatting changes improve consistency. The logic remains unchanged.
146-146: LGTM!The
_get_bootstrapmethod's logic remains unchanged and is appropriate.
150-151: LGTM!The
dumpmethod's string formatting changes improve consistency. The logic remains unchanged.
162-179: LGTM!The
set_recordmodemethod's string formatting changes improve consistency. The logic remains unchanged.
186-194: LGTM!The
get_recordmodemethod's string formatting changes improve consistency. The logic remains unchanged.
197-199: LGTM!The
get_picture_settingsmethod's string formatting changes improve consistency. The logic remains unchanged.
217-220: LGTM!The
prune_zonesmethod's string formatting changes improve consistency. The logic remains unchanged.
223-225: LGTM!The
list_zonesmethod's string formatting changes improve consistency. The logic remains unchanged.
232-243: LGTM!The
indexmethod's string formatting changes improve consistency. The logic remains unchanged.
254-256: LGTM!The
name_to_uuidmethod's string formatting changes improve consistency. The logic remains unchanged.
260-260: LGTM!The
get_cameramethod's string formatting changes improve consistency. The logic remains unchanged.
263-267: LGTM!The
get_snapshotmethod's string formatting changes improve consistency. The logic remains unchanged.
287-303: LGTM!The
get_auth_from_envfunction's string formatting changes improve consistency. The logic remains unchanged.
| with open(self._path, "w") as f: | ||
| f.write(base64.b64encode(json.dumps(self._data).encode()).decode()) | ||
| os.chmod(self._path, 0o600) | ||
| except (OSError, IOError) as ex: | ||
| LOG.error('Unable to write store: %s', str(ex)) | ||
| raise UnableToManageStore('Unable to write to store') | ||
| LOG.error("Unable to write store: %s", str(ex)) | ||
| raise UnableToManageStore("Unable to write to store") |
There was a problem hiding this comment.
Improve exception handling.
The string delimiter changes are approved. However, improve the exception handling by using raise ... from None.
- except (OSError, IOError) as ex:
- LOG.error("Unable to write store: %s", str(ex))
- raise UnableToManageStore("Unable to write to store")
+ except (OSError, IOError) as ex:
+ LOG.error("Unable to write store: %s", str(ex))
+ raise UnableToManageStore("Unable to write to store") from NoneCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with open(self._path, "w") as f: | |
| f.write(base64.b64encode(json.dumps(self._data).encode()).decode()) | |
| os.chmod(self._path, 0o600) | |
| except (OSError, IOError) as ex: | |
| LOG.error('Unable to write store: %s', str(ex)) | |
| raise UnableToManageStore('Unable to write to store') | |
| LOG.error("Unable to write store: %s", str(ex)) | |
| raise UnableToManageStore("Unable to write to store") | |
| with open(self._path, "w") as f: | |
| f.write(base64.b64encode(json.dumps(self._data).encode()).decode()) | |
| os.chmod(self._path, 0o600) | |
| except (OSError, IOError) as ex: | |
| LOG.error("Unable to write store: %s", str(ex)) | |
| raise UnableToManageStore("Unable to write to store") from None |
Tools
Ruff
39-39: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
| with open(self._path, "r") as f: | ||
| self._data = json.loads(base64.b64decode(f.read()).decode()) | ||
| except (OSError, IOError): | ||
| LOG.debug('No info store') | ||
| LOG.debug("No info store") | ||
| self._data = {} | ||
| except Exception as ex: | ||
| LOG.error('Failed to read store data: %s', ex) | ||
| raise UnableToManageStore('Unable to write to store') | ||
| LOG.error("Failed to read store data: %s", ex) | ||
| raise UnableToManageStore("Unable to write to store") |
There was a problem hiding this comment.
Improve exception handling.
The string delimiter changes are approved. However, improve the exception handling by using raise ... from None.
- except Exception as ex:
- LOG.error("Failed to read store data: %s", ex)
- raise UnableToManageStore("Unable to write to store")
+ except Exception as ex:
+ LOG.error("Failed to read store data: %s", ex)
+ raise UnableToManageStore("Unable to write to store") from NoneCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with open(self._path, "r") as f: | |
| self._data = json.loads(base64.b64decode(f.read()).decode()) | |
| except (OSError, IOError): | |
| LOG.debug('No info store') | |
| LOG.debug("No info store") | |
| self._data = {} | |
| except Exception as ex: | |
| LOG.error('Failed to read store data: %s', ex) | |
| raise UnableToManageStore('Unable to write to store') | |
| LOG.error("Failed to read store data: %s", ex) | |
| raise UnableToManageStore("Unable to write to store") | |
| with open(self._path, "r") as f: | |
| self._data = json.loads(base64.b64decode(f.read()).decode()) | |
| except (OSError, IOError): | |
| LOG.debug("No info store") | |
| self._data = {} | |
| except Exception as ex: | |
| LOG.error("Failed to read store data: %s", ex) | |
| raise UnableToManageStore("Unable to write to store") from None |
Tools
Ruff
30-30: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
uvcclient/camera.py
Outdated
| raise CameraConnectError("Unable to contact camera") | ||
| except httplib.HTTPException as ex: | ||
| raise CameraConnectError('Error connecting to camera: %s' % ( | ||
| str(ex))) | ||
| raise CameraConnectError("Error connecting to camera: %s" % (str(ex))) |
There was a problem hiding this comment.
Improve exception handling.
The string delimiter changes are approved. However, improve the exception handling by using raise ... from None.
- except (socket.error, OSError):
- raise CameraConnectError("Unable to contact camera")
- except httplib.HTTPException as ex:
- raise CameraConnectError("Error connecting to camera: %s" % (str(ex)))
+ except (socket.error, OSError):
+ raise CameraConnectError("Unable to contact camera") from None
+ except httplib.HTTPException as ex:
+ raise CameraConnectError("Error connecting to camera: %s" % (str(ex))) from NoneCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise CameraConnectError("Unable to contact camera") | |
| except httplib.HTTPException as ex: | |
| raise CameraConnectError('Error connecting to camera: %s' % ( | |
| str(ex))) | |
| raise CameraConnectError("Error connecting to camera: %s" % (str(ex))) | |
| raise CameraConnectError("Unable to contact camera") from None | |
| except httplib.HTTPException as ex: | |
| raise CameraConnectError("Error connecting to camera: %s" % (str(ex))) from None |
Tools
Ruff
55-55: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
57-57: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
uvcclient/nvr.py
Outdated
| conn = self._get_http_connection() | ||
| conn.request(*args, **kwargs) | ||
| return conn.getresponse() | ||
| except OSError: | ||
| raise CameraConnectionError('Unable to contact camera') | ||
| raise CameraConnectionError("Unable to contact camera") | ||
| except httplib.HTTPException as ex: | ||
| raise CameraConnectionError('Error connecting to camera: %s' % ( | ||
| str(ex))) | ||
| raise CameraConnectionError("Error connecting to camera: %s" % (str(ex))) |
There was a problem hiding this comment.
Improve error context in exception handling.
Use raise ... from err to provide better context for exceptions.
- except OSError:
- raise CameraConnectionError("Unable to contact camera")
- except httplib.HTTPException as ex:
- raise CameraConnectionError("Error connecting to camera: %s" % (str(ex)))
+ except OSError as err:
+ raise CameraConnectionError("Unable to contact camera") from err
+ except httplib.HTTPException as ex:
+ raise CameraConnectionError("Error connecting to camera: %s" % (str(ex))) from exCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| conn = self._get_http_connection() | |
| conn.request(*args, **kwargs) | |
| return conn.getresponse() | |
| except OSError: | |
| raise CameraConnectionError('Unable to contact camera') | |
| raise CameraConnectionError("Unable to contact camera") | |
| except httplib.HTTPException as ex: | |
| raise CameraConnectionError('Error connecting to camera: %s' % ( | |
| str(ex))) | |
| raise CameraConnectionError("Error connecting to camera: %s" % (str(ex))) | |
| conn = self._get_http_connection() | |
| conn.request(*args, **kwargs) | |
| return conn.getresponse() | |
| except OSError as err: | |
| raise CameraConnectionError("Unable to contact camera") from err | |
| except httplib.HTTPException as ex: | |
| raise CameraConnectionError("Error connecting to camera: %s" % (str(ex))) from ex |
Tools
Ruff
101-101: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
103-103: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
uvcclient/nvr.py
Outdated
| raise NvrError("Failed to contact NVR") | ||
| except httplib.HTTPException as ex: | ||
| raise NvrError('Error connecting to camera: %s' % str(ex)) | ||
|
|
||
| def _uvc_request_safe(self, path, method='GET', data=None, | ||
| mimetype='application/json'): | ||
| conn = self._get_http_connection(); | ||
| if '?' in path: | ||
| url = '%s&apiKey=%s' % (path, self._apikey) | ||
| raise NvrError("Error connecting to camera: %s" % str(ex)) |
There was a problem hiding this comment.
Improve error context in exception handling.
Use raise ... from err to provide better context for exceptions.
- except OSError:
- raise NvrError("Failed to contact NVR")
- except httplib.HTTPException as ex:
- raise NvrError("Error connecting to camera: %s" % str(ex))
+ except OSError as err:
+ raise NvrError("Failed to contact NVR") from err
+ except httplib.HTTPException as ex:
+ raise NvrError("Error connecting to camera: %s" % str(ex)) from exCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise NvrError("Failed to contact NVR") | |
| except httplib.HTTPException as ex: | |
| raise NvrError('Error connecting to camera: %s' % str(ex)) | |
| def _uvc_request_safe(self, path, method='GET', data=None, | |
| mimetype='application/json'): | |
| conn = self._get_http_connection(); | |
| if '?' in path: | |
| url = '%s&apiKey=%s' % (path, self._apikey) | |
| raise NvrError("Error connecting to camera: %s" % str(ex)) | |
| raise NvrError("Failed to contact NVR") from err | |
| except httplib.HTTPException as ex: | |
| raise NvrError("Error connecting to camera: %s" % str(ex)) from ex |
Tools
Ruff
109-109: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
111-111: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
uvcclient/nvr.py
Outdated
| url = "/api/2.0/camera/%s" % uuid | ||
| data = self._uvc_request(url) | ||
| for key in settings: | ||
| dtype = type(data['data'][0]['ispSettings'][key]) | ||
| dtype = type(data["data"][0]["ispSettings"][key]) | ||
| try: | ||
| data['data'][0]['ispSettings'][key] = dtype(settings[key]) | ||
| data["data"][0]["ispSettings"][key] = dtype(settings[key]) | ||
| except ValueError: | ||
| raise Invalid('Setting `%s\' requires %s not %s' % ( | ||
| key, dtype.__name__, type(settings[key]).__name__)) | ||
| data = self._uvc_request(url, 'PUT', json.dumps(data['data'][0])) | ||
| return data['data'][0]['ispSettings'] | ||
| raise Invalid( | ||
| "Setting `%s' requires %s not %s" | ||
| % (key, dtype.__name__, type(settings[key]).__name__) | ||
| ) |
There was a problem hiding this comment.
Improve error context in exception handling.
Use raise ... from err to provide better context for exceptions.
- except ValueError:
- raise Invalid(
- "Setting `%s' requires %s not %s"
- % (key, dtype.__name__, type(settings[key]).__name__)
- )
+ except ValueError as err:
+ raise Invalid(
+ "Setting `%s' requires %s not %s"
+ % (key, dtype.__name__, type(settings[key]).__name__)
+ ) from errCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| url = "/api/2.0/camera/%s" % uuid | |
| data = self._uvc_request(url) | |
| for key in settings: | |
| dtype = type(data['data'][0]['ispSettings'][key]) | |
| dtype = type(data["data"][0]["ispSettings"][key]) | |
| try: | |
| data['data'][0]['ispSettings'][key] = dtype(settings[key]) | |
| data["data"][0]["ispSettings"][key] = dtype(settings[key]) | |
| except ValueError: | |
| raise Invalid('Setting `%s\' requires %s not %s' % ( | |
| key, dtype.__name__, type(settings[key]).__name__)) | |
| data = self._uvc_request(url, 'PUT', json.dumps(data['data'][0])) | |
| return data['data'][0]['ispSettings'] | |
| raise Invalid( | |
| "Setting `%s' requires %s not %s" | |
| % (key, dtype.__name__, type(settings[key]).__name__) | |
| ) | |
| url = "/api/2.0/camera/%s" % uuid | |
| data = self._uvc_request(url) | |
| for key in settings: | |
| dtype = type(data["data"][0]["ispSettings"][key]) | |
| try: | |
| data["data"][0]["ispSettings"][key] = dtype(settings[key]) | |
| except ValueError as err: | |
| raise Invalid( | |
| "Setting `%s' requires %s not %s" | |
| % (key, dtype.__name__, type(settings[key]).__name__) | |
| ) from err |
Tools
Ruff
209-212: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Summary by CodeRabbit
New Features
CameraConnectionError, for improved error handling in camera operations.Style
Chores