Repository navigation
fix: pool settings propagation and qlty_rowsWithMissingValues tool fixes - #479
Merged
Merged
Conversation
…Teradata#474) The SQLAlchemy pool is lazy and opens no connections until the first tool call. In environments with stateful firewalls or NAT (e.g. GCP VMs), the initial TCP connection to Teradata stalls while the network path is established, causing the first pipeline request to hang indefinitely with no LogonOff entry recorded. Eagerly connect with SELECT 1 in the lifespan startup block so the cold-start hang surfaces at server startup rather than on the first client request, and ensures at least one pooled connection is ready before the server begins serving.
…_to_settings() Fixes issue Teradata#449: TD_POOL_SIZE, TD_MAX_OVERFLOW, and TD_POOL_TIMEOUT environment variables were read in settings_from_env() but then lost when parse_args_to_settings() created a new Settings instance without copying these fields. Also propagates other environment-only fields that were previously dropped: - ping_interval - auth_rate_limit_attempts - auth_rate_limit_window - registry_refresh_interval These fields now survive the transition from settings_from_env() to the final Settings object, ensuring that environment-based configuration is fully respected. Adds regression test in tests/phase0/test_pool_settings.py to prevent recurrence and verifies both custom and default values work correctly.
The TD_getRowsWithMissingValues function either doesn't exist or is not available in the Teradata installation, causing the qlty_rowsWithMissingValues tool to fail with "SSE stream ended without a response" errors. Replaced with a simpler, standard SQL approach using WHERE IS NULL to retrieve rows with missing values in the specified column. This approach: - Works on all Teradata systems without special function availability - Is simpler and more readable - Accomplishes the same goal more efficiently Fixes the qlty_rowsWithMissingValues test failures.
The tables_missing_comment test was using 'Tables' which either doesn't exist or doesn't have the CommentString column. Changed to 'TablesV' to match the Teradata system view naming convention and ensure the column exists. This makes the test case consistent with DBC system views that use the 'V' suffix (e.g., TablesV, DatabasesV, etc.).
…es test DBC.TablesV returns 1000 rows (hit the row cap), each containing a large RequestText column with DDL text. The response is too large for the SSE stream, causing 'SSE stream ended without a response'. DBC.ColumnsV has the same CommentString column for column-level comments but without large DDL text fields, keeping the response to a manageable size while still testing the same tool functionality.
…E responses SELECT * on wide catalog tables (e.g. TablesV with its RequestText DDL column) can return 1000 capped rows totalling many MB, causing the SSE stream to close before the response is transmitted. Adding TOP 50 keeps the response to a manageable size while still returning enough rows for the tool to be useful. Also restores the TablesV test case which is a realistic wide-table scenario.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
parse_args_to_settings()— resolves TD_POOL_SIZE, TD_MAX_OVERFLOW, and TD_POOL_TIMEOUT environment variables are ignored #449TD_getRowsWithMissingValuesTeradata function with standardSELECT … WHERE … IS NULLSQLTOP 50toqlty_rowsWithMissingValuesto prevent oversized SSE responses on wide catalog tablesDetails
Issue #449 — TD_POOL_SIZE / TD_MAX_OVERFLOW / TD_POOL_TIMEOUT ignored
parse_args_to_settings()inserver.pymerged CLI args with env settings but silently dropped several environment-only fields when constructing the finalSettingsobject. Affected fields:pool_size,max_overflow,pool_timeout(reported in TD_POOL_SIZE, TD_MAX_OVERFLOW, and TD_POOL_TIMEOUT environment variables are ignored #449)ping_interval,auth_rate_limit_attempts,auth_rate_limit_window,registry_refresh_interval(same pattern, fixed alongside)Fix: pass all missing fields through from
settings_from_env().A regression test was added in
tests/phase0/test_pool_settings.pycovering custom values, defaults, and the other previously-dropped env-only settings.qlty_rowsWithMissingValues tool
The tool SQL used
TD_getRowsWithMissingValues, a Teradata analytic function that is not available on all systems. Replaced with portable standard SQL:TOP 50prevents oversized SSE responses when the tool is called against wide catalog tables likeDBC.TablesV(which has a largeRequestTextDDL column). Without the limit, 1000 capped rows of DDL text can exhaust the SSE stream buffer.Test plan
python tests/phase0/test_pool_settings.py— all 3 assertions passqlty_rowsWithMissingValues(bothrows_missing_valuesandtables_missing_comment)TD_POOL_SIZE=37 TD_MAX_OVERFLOW=4 TD_POOL_TIMEOUT=91 uv run teradata-mcp-serverreflects custom pool settings at startup