SNOW-1674875: merge gate for multithreaded wrapper tests #2336
Conversation
…iables-thread-safe
…ection-thread-safe
…ad-safe' into aalam-SNOW-1418523-make-analyzer-server_connection-thread-safe
SNOW-1418523-make-udf-sproc-thread-safe
…n_stage, plan_builder
|
Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing |
…ub.com:snowflakedb/snowpark-python into SNOW-1720835-param-protect-client-side-changes
|
Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing |
| def session(): | ||
| with Session(MockServerConnection()) as s: | ||
| def mock_server_connection(multithreading_mode_enabled): | ||
| options = { |
There was a problem hiding this comment.
local testing does not make real connection so testing enabled/disabled mode using parameter.
…ded-wrapper-tests
|
Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing |
sfc-gh-aalam
added
the
NO-CHANGELOG-UPDATES
|
Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing |
| @@ -79,8 +79,12 @@ def auto_annotate_sql_counter(request): | |||
|
|
|||
| @pytest.fixture(autouse=True) | |||
| def check_sql_counter_invoked(request): | |||
| from tests.conftest import SKIP_SQL_COUNT_CHECK | |||
|
|
|||
There was a problem hiding this comment.
yes, right now yes. but we do not only develop for what will happen at this moment. it seems that with you change here, we need to propagate up this function to make sure the sql counter check can be adopted in general in snowpark python.
We don't have to do this in the current pr, but could you have a follow up pr to move that up to snowpark python for general usage
|
|
||
| @pytest.fixture(scope="session", autouse=True) | ||
| def multithreading_mode(pytestconfig): | ||
| enabled = pytestconfig.getoption("multithreading_mode") |
There was a problem hiding this comment.
sorry, what do you mean here? it seems this flag can only be turned on when the thread-safe session configuration can be turned on
| @pytest.mark.multithreaded_run | ||
| @functools.wraps(func) | ||
| def wrapper(*args, **kwargs): | ||
| if MULTITHREADING_TEST_MODE_ENABLED: |
There was a problem hiding this comment.
for those tests, probably doesn't work when the thread safe session object is not enabled, you might have to add a check here once you have the feature flag
There was a problem hiding this comment.
I seem to have removed config which creates session in thread-safe mode with --multithreading_mode option. I have readded it now.
|
Seems like your changes contain some Local Testing changes, please request review from @snowflakedb/local-testing |
sfc-gh-aalam
deleted the
aalam-SNOW-1674875-merge-gate-for-multithreaded-wrapper-tests
branch
Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1674875
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
This PR adds:
multithreading_modewhich runs tests marked withmultithreaded_runconcurrently in multiple threads using shared session.multithreaded_run