codeant-ai-for-open-source[bot] commented on code in PR #41250:
URL: https://github.com/apache/superset/pull/41250#discussion_r3443837821
##########
tests/integration_tests/email_tests.py:
##########
@@ -281,6 +287,26 @@ def test_send_mime_dryrun(self, mock_smtp, mock_smtp_ssl):
assert not mock_smtp.called
assert not mock_smtp_ssl.called
+ @mock.patch("smtplib.SMTP_SSL")
+ @mock.patch("smtplib.SMTP")
+ def test_send_mime_respects_custom_timeout(self, mock_smtp, mock_smtp_ssl):
+ # A missing timeout would block the worker forever when the SMTP server
+ # is unreachable, wedging the report schedule in WORKING (issue
#40047).
+ config = {**current_app.config, "SMTP_TIMEOUT": 7, "SMTP_SSL": False}
+ mock_smtp.return_value = mock.Mock()
+ utils.send_mime_email("from", "to", MIMEMultipart(), config,
dryrun=False)
+ assert mock_smtp.call_args.kwargs["timeout"] == 7
+
+ @mock.patch("smtplib.SMTP_SSL")
+ @mock.patch("smtplib.SMTP")
Review Comment:
**Suggestion:** Add explicit type annotations for the parameters and return
value in this newly introduced test method. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly introduced method has no parameter annotations or return
annotation. The rule explicitly requires new Python functions to be fully
typed, so this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=041d0dab198a474cbcc4fb9cb2709b1a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=041d0dab198a474cbcc4fb9cb2709b1a&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/email_tests.py
**Line:** 301:301
**Comment:**
*Custom Rule: Add explicit type annotations for the parameters and
return value in this newly introduced test method.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=dc3dfcfecd10027fed54f1a2e19ec677737ddea6da297262e0f612f7a48444fa&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=dc3dfcfecd10027fed54f1a2e19ec677737ddea6da297262e0f612f7a48444fa&reaction=dislike'>👎</a>
##########
tests/integration_tests/email_tests.py:
##########
@@ -281,6 +287,26 @@ def test_send_mime_dryrun(self, mock_smtp, mock_smtp_ssl):
assert not mock_smtp.called
assert not mock_smtp_ssl.called
+ @mock.patch("smtplib.SMTP_SSL")
+ @mock.patch("smtplib.SMTP")
Review Comment:
**Suggestion:** Add explicit type hints for all parameters and the return
type in this newly added test method. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python test method, and it has no type hints for its
parameters or return value. The custom rule requires new Python functions to be
fully typed, so the suggestion matches a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a87f2e0620c44ba0819feba4a8341830&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a87f2e0620c44ba0819feba4a8341830&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/email_tests.py
**Line:** 291:291
**Comment:**
*Custom Rule: Add explicit type hints for all parameters and the return
type in this newly added test method.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=e9ac9dbfd16b2c66aa187cf3c28488e3f1b4711b5de56c33ac21c6b7e435f2a9&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=e9ac9dbfd16b2c66aa187cf3c28488e3f1b4711b5de56c33ac21c6b7e435f2a9&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/csv_tests.py:
##########
@@ -75,19 +78,19 @@ def test_escape_value():
assert result == "'\rfoo"
-def fake_get_chart_csv_data_none(chart_url, auth_cookies=None):
+def fake_get_chart_csv_data_none(chart_url, auth_cookies=None, timeout=None):
return None
-def fake_get_chart_csv_data_empty(chart_url, auth_cookies=None):
+def fake_get_chart_csv_data_empty(chart_url, auth_cookies=None, timeout=None):
Review Comment:
**Suggestion:** Add explicit parameter and return type annotations for this
new helper function, including nullable types where appropriate. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly introduced helper function omits type hints for all parameters
and the return value, which matches the custom rule violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0421ee2c3c7f43db9e372e535e3eb1d6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0421ee2c3c7f43db9e372e535e3eb1d6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/csv_tests.py
**Line:** 85:85
**Comment:**
*Custom Rule: Add explicit parameter and return type annotations for
this new helper function, including nullable types where appropriate.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=2cb4e7ac1e99f8257fdc84095680e0ac068c18635d89b72e2164427666aff2f3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=2cb4e7ac1e99f8257fdc84095680e0ac068c18635d89b72e2164427666aff2f3&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/csv_tests.py:
##########
@@ -380,3 +383,35 @@ def test_get_chart_dataframe_preserves_na_string_values(
last_name_values = df[("last_name",)].values
assert last_name_values[0] == "Smith"
assert last_name_values[1] == "NA"
+
+
+def test_get_chart_csv_data_passes_timeout_to_opener(
+ monkeypatch: pytest.MonkeyPatch,
+):
+ # Without a timeout the request blocks forever when the webserver is
+ # unreachable, wedging the report schedule in WORKING (issue #40047).
+ mock_response = mock.Mock()
+ mock_response.read.return_value = b"data"
+ mock_response.getcode.return_value = 200
+ mock_opener = mock.Mock()
+ mock_opener.open.return_value = mock_response
+ mock_opener.addheaders = []
+ monkeypatch.setattr(
+ "urllib.request.build_opener", mock.Mock(return_value=mock_opener)
+ )
+
+ get_chart_csv_data("http://dummy-url";, auth_cookies={"session": "x"},
timeout=42)
+
+ mock_opener.open.assert_called_once_with("http://dummy-url";, timeout=42)
+
+
+def test_get_chart_dataframe_forwards_timeout(monkeypatch: pytest.MonkeyPatch):
+ captured = {}
+
+ def fake(chart_url, auth_cookies=None, timeout=None):
Review Comment:
**Suggestion:** Add type hints to this newly introduced nested function's
parameters and return value so all new function definitions remain fully typed.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This nested helper function is newly introduced and lacks annotations on its
parameters and return type, so it violates the fully-typed new Python code rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f6fe9fe8beae4ab29d953503c8c8a040&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f6fe9fe8beae4ab29d953503c8c8a040&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/csv_tests.py
**Line:** 411:411
**Comment:**
*Custom Rule: Add type hints to this newly introduced nested function's
parameters and return value so all new function definitions remain fully typed.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=5aa0b2b4348b700cd0638e20950336ea7469272108c163960c331f92c0b3e15c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=5aa0b2b4348b700cd0638e20950336ea7469272108c163960c331f92c0b3e15c&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/csv_tests.py:
##########
@@ -75,19 +78,19 @@ def test_escape_value():
assert result == "'\rfoo"
-def fake_get_chart_csv_data_none(chart_url, auth_cookies=None):
+def fake_get_chart_csv_data_none(chart_url, auth_cookies=None, timeout=None):
return None
-def fake_get_chart_csv_data_empty(chart_url, auth_cookies=None):
+def fake_get_chart_csv_data_empty(chart_url, auth_cookies=None, timeout=None):
# Return JSON with empty data so that the resulting DataFrame is empty
fake_result = {
"result": [{"data": {}, "coltypes": [], "colnames": [], "indexnames":
[]}]
}
return json.dumps(fake_result).encode("utf-8")
-def fake_get_chart_csv_data_valid(chart_url, auth_cookies=None):
+def fake_get_chart_csv_data_valid(chart_url, auth_cookies=None, timeout=None):
Review Comment:
**Suggestion:** Add complete type hints for this new function signature
(inputs and return type) to match the typed-code rule for changed Python code.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This new function definition is untyped, with missing annotations for its
parameters and return value, so the rule is actually violated.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=171aed5955d941e6995a9d403694ba3d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=171aed5955d941e6995a9d403694ba3d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/csv_tests.py
**Line:** 93:93
**Comment:**
*Custom Rule: Add complete type hints for this new function signature
(inputs and return type) to match the typed-code rule for changed Python code.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=d0c091fced4641fe5e62528e4f7001ea6f713a3195b36f6ec192114a40d7fff6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=d0c091fced4641fe5e62528e4f7001ea6f713a3195b36f6ec192114a40d7fff6&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/csv_tests.py:
##########
@@ -75,19 +78,19 @@ def test_escape_value():
assert result == "'\rfoo"
-def fake_get_chart_csv_data_none(chart_url, auth_cookies=None):
+def fake_get_chart_csv_data_none(chart_url, auth_cookies=None, timeout=None):
Review Comment:
**Suggestion:** Add explicit type hints to all parameters and the return
value for this new helper function so it complies with the fully-typed new
Python code requirement. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python helper function with no parameter annotations
and no return type hint, so it violates the rule that new Python code should be
fully typed.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b45404e33718474f9367a6a6c43c1632&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b45404e33718474f9367a6a6c43c1632&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/csv_tests.py
**Line:** 81:81
**Comment:**
*Custom Rule: Add explicit type hints to all parameters and the return
value for this new helper function so it complies with the fully-typed new
Python code requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=65f57df8f1524d0f96fbc7120fc7ed34e182b4a402ba95a9a4e53b101e9ad399&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=65f57df8f1524d0f96fbc7120fc7ed34e182b4a402ba95a9a4e53b101e9ad399&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/csv_tests.py:
##########
@@ -380,3 +383,35 @@ def test_get_chart_dataframe_preserves_na_string_values(
last_name_values = df[("last_name",)].values
assert last_name_values[0] == "Smith"
assert last_name_values[1] == "NA"
+
+
+def test_get_chart_csv_data_passes_timeout_to_opener(
+ monkeypatch: pytest.MonkeyPatch,
+):
Review Comment:
**Suggestion:** Add a docstring at the start of this newly added test
function describing the behavior being validated and why it matters.
[custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The newly added test function has a typed parameter, but it does not include
a docstring. The rule explicitly requires new functions and classes to be
documented inline.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d7c6a930bf4b42c29ce44a04b76b5860&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d7c6a930bf4b42c29ce44a04b76b5860&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/csv_tests.py
**Line:** 388:390
**Comment:**
*Custom Rule: Add a docstring at the start of this newly added test
function describing the behavior being validated and why it matters.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=e5b4e37f23bc727c0ff1ae42f9437224204f76303d08bc12192c5a39fa7a8164&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41250&comment_hash=e5b4e37f23bc727c0ff1ae42f9437224204f76303d08bc12192c5a39fa7a8164&reaction=dislike'>👎</a>
--
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]
