Skip to content

Refactor: reenable root_path as config parameter#311

Merged
99Lys merged 8 commits intodremio:mainfrom
99Lys:DX-111395-fix-object-storage-path-config-override
Dec 3, 2025
Merged

Refactor: reenable root_path as config parameter#311
99Lys merged 8 commits intodremio:mainfrom
99Lys:DX-111395-fix-object-storage-path-config-override

Conversation

@99Lys
Copy link
Copy Markdown
Contributor

@99Lys 99Lys commented Nov 19, 2025

Summary

Reenable root_path and respective aliases as config parameters.

Description

Currently, in order to set custom schemas for specific models, we use the config parameter schema (and respective aliases).

However, in the current implementation, dremio_space_folder / schema are both effective regardless of the materialization type (and object_storage_path / root_path were disabled). Since, in our adapter's context, schema typically belongs to views, it can be confusing to use only one term for all materialization types.

Therefore, in order to maintain consistency and be compliant with dbt standards, we will refactor how custom schemas can be set by following our internally defined aliases, i.e.:

  • schema and respective aliases - if used in config, will only be effective if the model is a view

  • root_path and respective aliases - if used in config, will only be effective if the model is a table

Both will behave the same way: append the value provided in config to the respective original one in profiles.yml.

Test Results

  • Refactored test test_nested_schema.py
  • Updated test_exact_search.py to use the correct config (replaced schema to root_path)

Changelog

  • Added a summary of what this PR accomplishes to CHANGELOG.md

@99Lys 99Lys force-pushed the DX-111395-fix-object-storage-path-config-override branch 2 times, most recently from 9d7ee9e to 8fa148f Compare November 25, 2025 20:10
@99Lys 99Lys marked this pull request as ready for review November 26, 2025 16:04
Copy link
Copy Markdown
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

This PR refactors the configuration parameter handling to differentiate between schema (for views) and root_path (for tables), making the adapter more consistent with dbt standards and less confusing for users.

Key changes:

  • root_path config now only applies to table materializations and is appended to the default root_path from profiles.yml
  • schema config now only applies to view materializations and is appended to the default schema from profiles.yml
  • Profile template hints updated to reflect the correct alias mappings

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/functional/adapter/dremio_specific/test_nested_schema.py Comprehensive test refactor with new tests validating that root_path works for tables and schema works for views, with tests confirming the wrong config is ignored
tests/functional/adapter/dremio_specific/test_exact_search.py Updated table model to use root_path config instead of schema
dbt/include/dremio/profile_template.yml Corrected alias hints: object_storage_path now aliases to root_path, dremio_space_folder now aliases to schema
dbt/include/dremio/macros/get_custom_name/get_custom_schema.sql Added logic to use node.config.root_path for datalake nodes (tables) instead of the generic custom_schema_name parameter
CHANGELOG.md Added entry documenting the reenabling of object_storage_path and root_path as config parameters

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Member

@simonpannek simonpannek left a comment

Choose a reason for hiding this comment

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

LGTM - if this is a breaking change, customers should probably be notified accordingly.
Also make sure to address the current test failures!

@99Lys 99Lys merged commit 95b9090 into dremio:main Dec 3, 2025
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants