codeant-ai-for-open-source[bot] commented on code in PR #38576:
URL: https://github.com/apache/superset/pull/38576#discussion_r3482432634
##########
tests/unit_tests/utils/test_screenshot_cache_fix.py:
##########
@@ -149,38 +155,37 @@ def test_cache_saved_only_when_image_generated(
assert cached_value["status"] == "Updated"
assert cached_value["image"] is not None
- def test_no_intermediate_cache_during_computing(
- self, mocker: MockerFixture, screenshot_obj, mock_user
- ):
- """Test that cache is not saved during COMPUTING state."""
+ def test_computing_status_written_to_cache_early(
+ self,
+ mocker: MockerFixture,
+ screenshot_obj: BaseScreenshot,
+ mock_user: MagicMock,
+ ) -> None:
+ """compute_and_cache writes COMPUTING to cache before taking the
screenshot
+ so concurrent tasks can detect it and avoid duplicate work."""
+ mocker.patch(DISTRIBUTED_LOCK_PATH)
mocker.patch(BASE_SCREENSHOT_PATH + ".get_from_cache_key",
return_value=None)
BaseScreenshot.cache = MockCache()
- # Mock get_screenshot to check cache state during execution
- def check_cache_during_screenshot(*args, **kwargs):
- # At this point, we're in COMPUTING state
- # Cache should NOT be set yet
+ def check_cache_during_screenshot(*args: object, **kwargs: object) ->
bytes:
cache_key = screenshot_obj.get_cache_key()
cached_value = BaseScreenshot.cache.get(cache_key)
- # Cache should be empty during screenshot generation
- assert cached_value is None, (
- "Cache should not be saved during COMPUTING state"
+ assert cached_value is not None, (
+ "Cache should be set to COMPUTING before screenshot starts"
)
+ assert cached_value["status"] == "Computing"
return b"image_data"
mocker.patch(
BASE_SCREENSHOT_PATH + ".get_screenshot",
side_effect=check_cache_during_screenshot,
)
- # Mock resize to avoid PIL errors with fake image data
mocker.patch(
BASE_SCREENSHOT_PATH + ".resize_image",
return_value=b"resized_image_data"
)
Review Comment:
**Suggestion:** Annotate all currently untyped parameters and add an
explicit return type for this modified integration test method. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The enclosing integration test `test_stale_computing_triggers_retry` is
modified and still has untyped parameters (`mock_app`, `mocker`,
`screenshot_obj`, `mock_user`) and no explicit return type. This violates the
custom rule requiring full typing on changed Python code.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4fe69041c42547b2b0208e045e67d812&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=4fe69041c42547b2b0208e045e67d812&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/test_screenshot_cache_fix.py
**Line:** 176:185
**Comment:**
*Custom Rule: Annotate all currently untyped parameters and add an
explicit return type for this modified integration 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%2F38576&comment_hash=c1f47272315cf5e17cb7f463471f92d66073399dd9e6e7a94ca97640a982eacf&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38576&comment_hash=c1f47272315cf5e17cb7f463471f92d66073399dd9e6e7a94ca97640a982eacf&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]
