-
Notifications
You must be signed in to change notification settings - Fork 2
1.9.x to main #143
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
1.9.x to main #143
Changes from all commits
cecb4be
0655e28
8c46dde
d0dced7
5a4525d
8ae6eda
b0043ae
1e0097b
40a6fe9
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 |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| """Version details for willisapi_client""" | ||
|
|
||
| __client__ = "willisapi_client" | ||
| __latestVersion__ = "1.9.6" | ||
| __latestVersion__ = "1.9.7" | ||
| __url__ = "https://github.com/bklynhlth/willsiapi_client" | ||
| __short_description__ = "A Python client for willisapi" | ||
| __content_type__ = "text/markdown" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| import os | ||
| import requests | ||
|
|
||
| from willisapi_client.willisapi_client import WillisapiClient | ||
| from willisapi_client.logging_setup import logger as logger | ||
|
|
||
|
|
||
| def _archive_headers(api_key): | ||
| return { | ||
| "Content-Type": "application/json", | ||
| "Accept": "application/json", | ||
| "Authorization": f"token {api_key}", | ||
| } | ||
|
|
||
|
|
||
| def archive_metadata_csv(api_key, csv_path, total_rows, upload_type, env=None): | ||
| """Archive the source metadata CSV alongside the data upload. | ||
|
|
||
| Creates a server-side tracking row, then PUTs the raw CSV to the returned | ||
| presigned S3 URL. Returns the tracking ``record_id`` on success (so the | ||
| caller can finalize it with row counts), or ``None`` on failure. | ||
|
|
||
| All failures are logged and swallowed — archiving must never abort the | ||
| actual data upload. | ||
| """ | ||
| try: | ||
| wc = WillisapiClient(env=env) | ||
| payload = { | ||
| "filename": os.path.basename(csv_path), | ||
| "total_rows": total_rows, | ||
| "upload_type": upload_type, | ||
| } | ||
| res = requests.post( | ||
| wc.get_csv_archive_url(), headers=_archive_headers(api_key), json=payload | ||
| ) | ||
| if res.status_code != 201: | ||
| logger.warning(f"CSV archive init failed: {res.status_code} {res.text}") | ||
| return None | ||
|
|
||
| body = res.json() | ||
| record_id = body.get("record_id") | ||
| presigned = body.get("presigned_url") | ||
| if not presigned: | ||
| logger.warning("CSV archive: no presigned URL returned") | ||
| return None | ||
|
|
||
| with open(csv_path, "rb") as f: | ||
| put_res = requests.put( | ||
| presigned, data=f, headers={"Content-Type": "text/csv"} | ||
| ) | ||
|
Comment on lines
+48
to
+50
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. Add timeout to prevent indefinite hangs. The ⏱️ Proposed fix to add timeout put_res = requests.put(
- presigned, data=f, headers={"Content-Type": "text/csv"}
+ presigned,
+ data=f,
+ headers={"Content-Type": "text/csv"},
+ timeout=60
)🧰 Tools🪛 Ruff (0.15.15)[error] 48-48: Probable use of (S113) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
| if put_res.status_code not in (200, 204): | ||
| logger.warning(f"CSV archive S3 upload failed: {put_res.status_code}") | ||
| finalize_metadata_csv(api_key, record_id, "failed", 0, 0, env=env) | ||
| return None | ||
|
|
||
| return record_id | ||
| except Exception as ex: | ||
| logger.warning(f"CSV archive error: {ex}") | ||
| return None | ||
|
|
||
|
|
||
| def finalize_metadata_csv( | ||
| api_key, record_id, status, successful_rows, failed_rows, env=None | ||
| ): | ||
| """Report the CSV archive outcome and parsed row counts to the server.""" | ||
| if not record_id: | ||
| return | ||
| try: | ||
| wc = WillisapiClient(env=env) | ||
| payload = { | ||
| "status": status, | ||
| "successful_rows": successful_rows, | ||
| "failed_rows": failed_rows, | ||
| } | ||
| res = requests.patch( | ||
| wc.get_csv_archive_finalize_url(record_id), | ||
| headers=_archive_headers(api_key), | ||
| json=payload, | ||
| ) | ||
|
vjbytes102 marked this conversation as resolved.
|
||
| if res.status_code != 200: | ||
| logger.warning(f"CSV archive finalize failed: {res.status_code} {res.text}") | ||
| except Exception as ex: | ||
| logger.warning(f"CSV archive finalize error: {ex}") | ||
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.
Add timeout to prevent indefinite hangs.
The
requests.postcall lacks a timeout parameter. Without a timeout, this network call could hang indefinitely, violating the documented requirement that "archiving must never abort the actual data upload."⏱️ Proposed fix to add timeout
📝 Committable suggestion
🧰 Tools
🪛 Ruff (0.15.15)
[error] 33-33: Probable use of
requestscall without timeout(S113)
🤖 Prompt for AI Agents
Source: Linters/SAST tools