Skip to content

Validate client session keep alive heartbeat frequency type fix#2839

Merged
sfc-gh-fpawlowski merged 4 commits intosnowflakedb:mainfrom
abdulselamadillmohammed:validate_client_session_keep_alive_heartbeat_frequency_type_fix
Apr 9, 2026
Merged

Validate client session keep alive heartbeat frequency type fix#2839
sfc-gh-fpawlowski merged 4 commits intosnowflakedb:mainfrom
abdulselamadillmohammed:validate_client_session_keep_alive_heartbeat_frequency_type_fix

Conversation

@abdulselamadillmohammed
Copy link
Copy Markdown
Contributor

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-3311068: _validate_client_session_keep_alive_heartbeat_frequency handles None incorrectly due to int() cast order #2837

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am modifying authorization mechanisms
    • I am adding new credentials
    • I am modifying OCSP code
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

This change fixes the validation order in _validate_client_session_keep_alive_heartbeat_frequency(). The validator now handles None before attempting integer conversion, which restores the intended fallback behavior and avoids raising TypeError when None is assigned through the Python setter path. It preserves conversion of non-None values such as "10" to integers and keeps the existing range clamping behavior.

  1. (Optional) PR for stored-proc connector:

N/A

@abdulselamadillmohammed abdulselamadillmohammed requested a review from a team as a code owner March 31, 2026 14:46
@sfc-gh-turbaszek
Copy link
Copy Markdown
Contributor

@sfc-gh-fpawlowski can you please review?

Copy link
Copy Markdown
Contributor

@sfc-gh-fpawlowski sfc-gh-fpawlowski left a comment

Choose a reason for hiding this comment

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

Looks good to me, left one small comment and started workflows. If they pass we can merge this PR 👍

value = self.client_session_keep_alive_heartbeat_frequency

if self.client_session_keep_alive_heartbeat_frequency is None:
# This is an unlikely scenario but covering it just in case.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: We may want to leave this comment for reference.

@sfc-gh-fpawlowski sfc-gh-fpawlowski enabled auto-merge (squash) April 9, 2026 08:22
Added comment removed before
Copy link
Copy Markdown
Contributor

@sfc-gh-fpawlowski sfc-gh-fpawlowski left a comment

Choose a reason for hiding this comment

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

Fixed comment, lgtm 👍

@sfc-gh-fpawlowski sfc-gh-fpawlowski merged commit fd04861 into snowflakedb:main Apr 9, 2026
69 of 83 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SNOW-3311068: _validate_client_session_keep_alive_heartbeat_frequency handles None incorrectly due to int() cast order

3 participants