Skip to content

feat: add named secret support DuckDB#4912

Merged
eakmanrq merged 1 commit intomainfrom
eakmanrq/add_named_secret_support_duckdb
Jul 4, 2025
Merged

feat: add named secret support DuckDB#4912
eakmanrq merged 1 commit intomainfrom
eakmanrq/add_named_secret_support_duckdb

Conversation

@eakmanrq
Copy link
Contributor

@eakmanrq eakmanrq commented Jul 4, 2025

Prior to this PR, users were forced to use the default secret name when creating secrets. Now they can still do that or create secrets with a specific name. This is required to support a use case where users want to define multiple secrets for a single type (ex: multiple secrets for S3) since before they would get an error saying the default name was already defined.

@eakmanrq eakmanrq force-pushed the eakmanrq/add_named_secret_support_duckdb branch 2 times, most recently from 279eeeb to 670997d Compare July 4, 2025 16:23
@eakmanrq eakmanrq requested a review from Copilot July 4, 2025 16:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for named secrets in DuckDB connections, allowing multiple secrets per connection either as a list or a dict with custom names.

  • Extends the secrets config type and initialization to handle dict-based named secrets.
  • Updates tests to cover both list and dict formats for DuckDB secrets.
  • Revises examples and documentation to illustrate named and default secret usage.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tests/core/test_connection_config.py Added tests for multiple and named DuckDB secrets
sqlmesh/core/config/connection.py Changed secrets field type and init logic for naming
examples/sushi/config.py Updated example to show named secret configuration
docs/integrations/engines/duckdb.md Expanded docs with list vs. dict formats for secrets
Comments suppressed due to low confidence (3)

tests/core/test_connection_config.py:7

  • The test uses call in assertions but does not import it from unittest.mock; add call to the import to avoid a NameError.
from unittest.mock import patch, MagicMock

sqlmesh/core/config/connection.py:280

  • Using a mutable default for the secrets field can lead to shared state issues; consider using Field(default_factory=list) (or equivalent) for list defaults or default_factory=dict when appropriate.
    secrets: t.Union[t.List[t.Dict[str, t.Any]], t.Dict[str, t.Dict[str, t.Any]]] = []

sqlmesh/core/config/connection.py:380

  • [nitpick] When secret_name is empty, this generates an extra space in the SQL (CREATE SECRET ...); consider conditionally adding the space only when a name is provided.
                                cursor.execute(f"CREATE SECRET {secret_name} ({secret_clause});")

Comment on lines +354 to +356
type: ducklake
path: myducklakecatalog.duckdb
data_path: abfs://MyFabricWorkspace/MyFabricLakehouse.Lakehouse/Files/DuckLake.Files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a typo I found while reviewing the docs after the change in this PR

@eakmanrq eakmanrq force-pushed the eakmanrq/add_named_secret_support_duckdb branch from 670997d to 0b2c602 Compare July 4, 2025 16:34
@eakmanrq eakmanrq merged commit 3b66046 into main Jul 4, 2025
27 checks passed
@eakmanrq eakmanrq deleted the eakmanrq/add_named_secret_support_duckdb branch July 4, 2025 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants