aminghadersohi commented on code in PR #43977:
URL: https://github.com/apache/superset/pull/43977#discussion_r3953943052
##########
superset/utils/csv.py:
##########
@@ -119,10 +120,10 @@ def get_chart_csv_data(
opener.addheaders.append(("Cookie", cookie_str))
# A missing timeout means the socket blocks forever when the Superset
# webserver is unreachable, wedging the report schedule in WORKING.
- response = opener.open(chart_url, timeout=timeout)
- content = response.read()
- if response.getcode() != 200:
- raise URLError(response.getcode())
+ with closing(opener.open(chart_url, timeout=timeout)) as response:
+ content = response.read()
+ if response.getcode() != 200:
+ raise URLError(response.getcode())
Review Comment:
Nit, pre-existing in shape — but this PR is what makes it observable, so
worth one line.
`URLError(response.getcode())` builds a `URLError` whose `.reason` is an
**`int`**. In `chart_data.py` the handler does `reason = error.reason if
isinstance(error, URLError) else error`, so `reason` becomes that int: not a
`TimeoutError`, not an `IncompleteRead`, no `.errno`. It therefore classifies
as `category="network"`, `transient=False`, `status=None` — and the actual HTTP
code is dropped from both the warning log line and the raised
`ChartDataRequestError`, on a PR whose stated purpose is transport diagnostics.
Reachability keeps this genuinely small: `build_opener()` installs
`HTTPErrorProcessor`, which already raises a real `HTTPError` for anything
non-2xx, so this branch can only fire on an unexpected 2xx (204/206). Rare
enough that I would not restructure `csv.py` for it — and passing a synthetic
`HTTPError` up instead would hand `_error_body` an `fp=None` to `read()`, which
its `except` tuple does not cover, so that is not the fix either. If you want
the code preserved, the cheapest place is a small `isinstance(reason, int)`
case in the classifier. Entirely your call whether it earns the line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
