Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the Agate client from using PyCurl to Python’s requests library.
- Replaces PyCurl calls with requests in client, request, and response handling.
- Updates tests to include the new no_ssl_verify parameter and remove certificate validation in one test.
- Adjusts dependency management and command-line arguments accordingly.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_restCall.py | Updates authentication calls to pass no_ssl_verify and removes certificate usage. |
| pyproject.toml | Removes the pycurl dependency and adds requests and urllib3 dependencies. |
| obiba_agate/rest.py | Wraps REST command execution with a try/finally block to ensure client closure. |
| obiba_agate/core.py | Replaces PyCurl usage with requests, updates header handling, and SSL verification. |
| obiba_agate/console.py | Adds a new command-line argument to disable SSL certificate verification. |
|
|
||
| def content_type(self, value): | ||
| return self.header('Content-Type', value) | ||
| return self.headers.update({"Content-Type": value}) |
There was a problem hiding this comment.
The content_type method currently returns the result of dict.update, which is None, breaking the method chaining pattern. Consider updating it to call self.header('Content-Type', value) or explicitly return self.
| return self.headers.update({"Content-Type": value}) | |
| return self.header('Content-Type', value) |
| [info["major"], info["minor"], info["patch"]] = self.version.split(".") | ||
| return info |
There was a problem hiding this comment.
[nitpick] The direct destructuring assignment assumes the version string always splits into three parts. Consider adding validation or error handling to avoid potential issues with unexpected version formats.
| [info["major"], info["minor"], info["patch"]] = self.version.split(".") | |
| return info | |
| version_parts = self.version.split(".") | |
| if len(version_parts) == 3: | |
| info["major"], info["minor"], info["patch"] = version_parts | |
| return info | |
| else: | |
| # Handle malformed version string | |
| return None |
| self.curl_option(pycurl.HTTPPOST, [("file1", (pycurl.FORM_FILE, filename))]) | ||
| print("* File Content:") | ||
| print("[file=" + filename + ", size=" + str(os.path.getsize(filename)) + "]") | ||
| self.files = {"file": (filename, open(filename, "rb"))} |
There was a problem hiding this comment.
The file opened for upload is not explicitly closed after use. Consider using a context manager or ensuring the file is properly closed to avoid resource leaks.
| self.files = {"file": (filename, open(filename, "rb"))} | |
| with open(filename, "rb") as file: | |
| self.files = {"file": (filename, file.read())} |
No description provided.