Feature/catch notsupported exception

[snickerjp]

What type of PR is this?

• Refactor
• Bug Fix

Description

This PR implements an alternative approach to #7572 by catching NotSupported exceptions in refresh_schema() instead of pre-filtering datasource types.

Addresses issue #7571 - Clean up error logs from datasources that don't support schema refresh.

Motivation

As suggested by @yoshiokatsuneo in PR #7572 review, this approach is simpler and more maintainable:

• No global configuration needed
• Automatically handles all unsupported datasources
• Cleaner separation of concerns

Changes

• Add NotSupported exception handling to refresh_schema()
• Log unsupported datasources at DEBUG level
• Avoid incrementing error metrics for datasources without schema support

Files changed:

• redash/tasks/queries/maintenance.py: +3 lines (1 import, 2 exception handling)
• tests/tasks/test_refresh_schemas.py: +10 lines (1 test case)

Flow Diagram

flowchart TD
    Start([refresh_schemas]) --> Loop{For each<br/>datasource}
    Loop -->|Paused| Skip1[Skip: paused]
    Loop -->|Active| Queue[Enqueue refresh_schema job]
    
    Queue --> Worker[Worker executes job]
    Worker --> Try[Try: ds.get_schema refresh=True]
    
    Try -->|Success| Success[Log: state=finished<br/>Increment: success metric]
    Try -->|JobTimeoutException| Timeout[Log: state=timeout<br/>Increment: timeout metric]
    Try -->|NotSupported| NotSupp[Log DEBUG: not supported<br/>No metric increment]
    Try -->|Other Exception| Error[Log WARNING: failed<br/>Increment: error metric]
    
    Success --> JobOK1[Job OK]
    Timeout --> JobOK2[Job OK]
    NotSupp --> JobOK3[Job OK]
    Error --> JobOK4[Job OK]
    
    Skip1 --> Loop
    JobOK1 --> Loop
    JobOK2 --> Loop
    JobOK3 --> Loop
    JobOK4 --> Loop
    
    Loop -->|Done| End([End])
    
    style NotSupp fill:#2d5016,stroke:#4a7c2c,color:#fff
    style Success fill:#1e4d7b,stroke:#2e6da4,color:#fff
    style Error fill:#8b2e3b,stroke:#c9302c,color:#fff
    style Timeout fill:#8b6914,stroke:#d9a441,color:#fff

Loading

Key Points:

• 🟢 NotSupported: Silently handled at DEBUG level, no error metrics
• 🔵 Success: Normal completion with metrics
• 🔴 Error: Other exceptions logged as warnings with metrics
• 🟡 Timeout: Job timeout logged with metrics

How is this tested?

• Unit tests (pytest, jest)
• Manually

Unit Tests

$ docker compose run --rm server pytest tests/tasks/test_refresh_schemas.py -v
========================== test session starts ===========================
collected 3 items

tests/tasks/test_refresh_schemas.py::TestRefreshSchemas::test_calls_refresh_of_all_data_sources PASSED [ 33%]
tests/tasks/test_refresh_schemas.py::TestRefreshSchemas::test_skips_paused_data_sources PASSED [ 66%]
tests/tasks/test_refresh_schemas.py::TestRefreshSchemas::test_handles_notsupported_exception PASSED [100%]

========================== 3 passed in 8.41s =============================

Manual Testing

Tested with 3 datasources:

• Query Results (type=results, ds_id=1) - NotSupported exception caught ✅
• Python (type=python, ds_id=2) - NotSupported exception caught ✅
• PostgreSQL (type=pg, ds_id=3) - Schema refresh succeeded ✅

Worker logs (INFO level):

[INFO] task=refresh_schema state=start ds_id=1
[INFO] schemas: Job OK (...)
[INFO] task=refresh_schema state=start ds_id=2
[INFO] schemas: Job OK (...)
[INFO] task=refresh_schema state=start ds_id=3
[INFO] task=refresh_schema state=finished ds_id=3 runtime=0.02
[INFO] schemas: Job OK (...)

No error logs, all jobs completed successfully.

DEBUG Log Verification

When DEBUG level is enabled, unsupported datasources are logged:

[DEBUG] Datasource Query Results does not support schema refresh

Comparison with PR #7572

Aspect	This PR (Exception Handling)	PR #7572 (Type Exclusion)
Configuration	None required	REDASH_SCHEMAS_REFRESH_EXCLUDED_TYPES
Code changes	3 lines	7 lines
Maintenance	Automatic	Manual type list
Job execution	All datasources	Filtered datasources
Flexibility	Handles all NotSupported	Only configured types

Benefits

1. Simpler: No configuration needed
2. Automatic: Works for all datasources that raise NotSupported
3. Future-proof: New datasources automatically handled
4. Clean logs: No error messages in production (DEBUG level only)

Log Output Behavior

• DEBUG logs not visible by default (achieves the primary goal of Issue Schema Refresh Fails for Certain Datasource Types #7571: "clean up error logs")

Related Tickets & Documents

• Issue: Schema Refresh Fails for Certain Datasource Types #7571
• Alternative approach: Add Datasource Type Exclusion from Schema Refresh #7572
• Suggested by: @yoshiokatsuneo

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

N/A - Backend changes only

[yoshiokatsuneo]

@snickerjp

Thank you for your PR!

I just feel that the test is too much as the test is testing only logging for this 3 lines of code.
I think, If we add tests for every logging like this in whole redash code base, I feel it does not improve the code quality but may rather make it difficult to read or maintain the whole code or tests.

How about ?

[yoshiokatsuneo]

@snickerjp

And, this is just my curious. And, I'm sorry if it is my mistaken.
May I ask whether you are using AI agent to create the PR, code, and/or tests ?
(Again, I'm not against it(I also sometimes uses AI), but it may make it easy to understand the context as AI agents sometimes makes more code than the human.)

[Copilot]

Pull request overview

This PR implements cleaner error handling for datasources that don't support schema refresh by catching NotSupported exceptions and logging them at DEBUG level instead of WARNING level. This approach is simpler and more maintainable than the alternative of pre-filtering datasource types.

Key changes:

• Added NotSupported exception handler in refresh_schema() task that logs at DEBUG level without incrementing error metrics
• Added unit test to verify the NotSupported exception is handled correctly
• Maintains exception ordering (NotSupported caught before generic Exception)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
redash/tasks/queries/maintenance.py	Added NotSupported import and exception handler that logs at DEBUG level without incrementing error metrics
tests/tasks/test_refresh_schemas.py	Added test case to verify NotSupported exceptions are logged at DEBUG level and don't trigger warning logs

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test should verify that no error metric is incremented when NotSupported exception is raised. This is a key behavior that distinguishes NotSupported from other exceptions (line 185 increments the error metric for general exceptions, but line 181-182 should not increment any metric). Consider adding an assertion to check that statsd_client.incr was not called with "refresh_schema.error".

[snickerjp]

@yoshiokatsuneo

Thank you for the feedback!

Regarding the test: I've removed the added test as you suggested. You're absolutely right that testing logging details for 3 lines of code is excessive.

Regarding AI usage: Yes, I am using AI assistance. The test was added based on my instruction, following the PR template's "How is this tested?" section which includes "Unit tests (pytest, jest)" as an option. I wanted to ensure comprehensive testing coverage, but I understand now that it was overkill for this minimal change.

The PR now contains only the essential 3-line change to handle NotSupported exceptions.

[yoshiokatsuneo]

@snickerjp

Thank you for updated PR, and the detailed comment.
I confirmed that there is no issue on schema refresh in PostgreSQL and Query Result data source.
I think the PR is just quite simple and make Redash better.
I just approved the PR.

Thank you !

[yoshiokatsuneo]

@snickerjp

(Now, I think you can just click "Squash and Merge" button whenever you want to merge. )