Skip to content

NO-SNOW: stop relying on connector threads in pandas CTE test - #4324

Merged
sfc-gh-bkogan merged 2 commits into
mainfrom
bkogan/fix-pandas-cte-thread-test
Aug 20, 2026
Merged

NO-SNOW: stop relying on connector threads in pandas CTE test#4324
sfc-gh-bkogan merged 2 commits into
mainfrom
bkogan/fix-pandas-cte-thread-test

Conversation

@sfc-gh-bkogan

Copy link
Copy Markdown
Collaborator

Summary

  • Daily Snowpark pandas CI (test_cte_optimization_for_snowpark_pandas) has been failing on Linux/macOS since ~Aug 12 because create_dataframe no longer leaves a connector background thread around.
  • The test only needed another live thread so warn_session_config_update_in_multithreaded_mode would fire; it now starts a dummy thread instead of depending on connector internals.
  • Windows jobs were already passing because extra threads exist there anyway.

Test plan

  • pytest tests/integ/modin/test_session_cte_optimization.py::test_cte_optimization_for_snowpark_pandas locally in the snowflake conda env (passed)
  • Daily Snowpark pandas API job should go green for this test on Linux/macOS

Daily Snowpark pandas CI failed because create_dataframe no longer leaves a background thread on Linux/macOS. Spawn a dummy thread so the warning-filter assertion is deterministic.

@sfc-gh-bkogan sfc-gh-bkogan left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHIP WITH NITS

Correct, test-only change that decouples the multithreading-warning trigger from connector internals and is more reliable than the old create_dataframe dependency. The suppression path is still genuinely exercised because the probe thread guarantees active_count() > 1 at the moment cte_optimization_enabled flips to True. A few optional robustness nits are inline.

Comment thread tests/integ/modin/test_session_cte_optimization.py Outdated
Comment thread tests/integ/modin/test_session_cte_optimization.py Outdated
Comment thread tests/integ/modin/test_session_cte_optimization.py Outdated
…thread join

Address review nits: add a positive control for the multithreading warning, drop the redundant daemon flag, and fail if the probe thread does not stop.
@sfc-gh-bkogan sfc-gh-bkogan added the NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md label Aug 20, 2026
@sfc-gh-bkogan
sfc-gh-bkogan marked this pull request as ready for review August 20, 2026 18:06
@sfc-gh-bkogan
sfc-gh-bkogan requested a review from a team as a code owner August 20, 2026 18:06

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@snowflake-security-bot snowflake-security-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snowflake Security Review

Security grade: A — Passed

This PR was classified as LOW risk by the automated pre-screen.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.46%. Comparing base (6ab27ed) to head (267cce5).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4324      +/-   ##
==========================================
+ Coverage   95.24%   95.46%   +0.22%     
==========================================
  Files         171      171              
  Lines       44720    44720              
  Branches     7676     7676              
==========================================
+ Hits        42593    42694     +101     
+ Misses       1340     1253      -87     
+ Partials      787      773      -14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sfc-gh-bkogan
sfc-gh-bkogan merged commit 10604a6 into main Aug 20, 2026
35 of 37 checks passed
@sfc-gh-bkogan
sfc-gh-bkogan deleted the bkogan/fix-pandas-cte-thread-test branch August 20, 2026 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md snowpark-pandas

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants