Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exposure parsing to allow other resources with the same name #3009

Merged
merged 3 commits into from
Jan 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## dbt 0.19.0 (Release TBD)

### Fixes
- Fix regression with defining exposures and other resources with the same name ([#2969](https://github.com/fishtown-analytics/dbt/issues/2969), [#3009](https://github.com/fishtown-analytics/dbt/pull/3009))

### Under the hood
- Rewrite macro for snapshot_merge_sql to make compatible with other SQL dialects ([#3003](https://github.com/fishtown-analytics/dbt/pull/3003)
- Rewrite logic in `snapshot_check_strategy()` to make compatible with other SQL dialects ([#3000](https://github.com/fishtown-analytics/dbt/pull/3000), [#3001](https://github.com/fishtown-analytics/dbt/pull/3001))
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ class ExposureOwner(JsonSchemaMixin, Replaceable):


@dataclass
class UnparsedExposure(HasYamlMetadata, Replaceable):
class UnparsedExposure(JsonSchemaMixin, Replaceable):
name: str
type: ExposureType
owner: ExposureOwner
Expand Down
5 changes: 5 additions & 0 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ def error_context(

class ParserRef:
"""A helper object to hold parse-time references."""

def __init__(self):
self.column_info: Dict[str, ColumnInfo] = {}

Expand Down Expand Up @@ -584,6 +585,10 @@ def parse_file(self, block: FileBlock) -> None:
parser = MacroPatchParser(self, yaml_block, plural)
elif key == NodeType.Analysis:
parser = AnalysisPatchParser(self, yaml_block, plural)
elif key == NodeType.Exposure:
# handle exposures separately, but they are
# technically still "documentable"
continue
else:
parser = TestablePatchParser(self, yaml_block, plural)
for test_block in parser.parse():
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
version: 2
exposures:
- name: something
type: dashboard
owner:
email: test@example.com
- name: something
type: dashboard
owner:
email: test@example.com
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version: 2
models:
- name: something
description: This table has basic information about orders, as well as some derived facts based on payments
exposures:
- name: something
type: dashboard
owner:
email: test@example.com
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
from dbt.exceptions import CompilationException
from test.integration.base import DBTIntegrationTest, use_profile


class TestDuplicateExposure(DBTIntegrationTest):

@property
def schema(self):
return "duplicate_exposure_025"

@property
def models(self):
return "models-exposure-dupes"

@use_profile("postgres")
def test_postgres_duplicate_exposure(self):
message = "dbt found two resources with the name"
try:
self.run_dbt(["compile"])
self.assertTrue(False, "dbt did not throw for duplicate exposures")
except CompilationException as e:
self.assertTrue(message in str(
e), "dbt did not throw the correct error message")
46 changes: 4 additions & 42 deletions test/integration/025_duplicate_model_test/test_duplicate_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,15 @@ def schema(self):
def models(self):
return "models-1"

@property
def profile_config(self):
return {
"test": {
"outputs": {
"dev": {
"type": "postgres",
"threads": 1,
"host": self.database_host,
"port": 5432,
"user": "root",
"pass": "password",
"dbname": "dbt",
"schema": self.unique_schema()
},
},
"target": "dev"
}
}

@use_profile("postgres")
def test_postgres_duplicate_model_enabled(self):
message = "dbt found two resources with the name"
try:
self.run_dbt(["run"])
self.assertTrue(False, "dbt did not throw for duplicate models")
except CompilationException as e:
self.assertTrue(message in str(e), "dbt did not throw the correct error message")
self.assertTrue(message in str(
e), "dbt did not throw the correct error message")


class TestDuplicateModelDisabled(DBTIntegrationTest):
Expand All @@ -52,26 +33,6 @@ def schema(self):
def models(self):
return "models-2"

@property
def profile_config(self):
return {
"test": {
"outputs": {
"dev": {
"type": "postgres",
"threads": 1,
"host": self.database_host,
"port": 5432,
"user": "root",
"pass": "password",
"dbname": "dbt",
"schema": self.unique_schema()
},
},
"target": "dev"
}
}

@use_profile("postgres")
def test_postgres_duplicate_model_disabled(self):
try:
Expand Down Expand Up @@ -124,7 +85,8 @@ def test_postgres_duplicate_model_enabled_across_packages(self):
self.run_dbt(["run"])
self.assertTrue(False, "dbt did not throw for duplicate models")
except CompilationException as e:
self.assertTrue(message in str(e), "dbt did not throw the correct error message")
self.assertTrue(message in str(
e), "dbt did not throw the correct error message")


class TestDuplicateModelDisabledAcrossPackages(DBTIntegrationTest):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from dbt.exceptions import CompilationException
from test.integration.base import DBTIntegrationTest, use_profile


class TestDuplicateSchemaResource(DBTIntegrationTest):

@property
def schema(self):
return "duplicate_resource_025"

@property
def models(self):
return "models-naming-dupes-1"

@use_profile("postgres")
def test_postgres_duplicate_model_and_exposure(self):
try:
self.run_dbt(["compile"])
except CompilationException:
self.fail("Compilation Exception raised on model and "
"exposure with the same name")
25 changes: 3 additions & 22 deletions test/integration/025_duplicate_model_test/test_duplicate_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,12 @@ def schema(self):
def models(self):
return "models-source-dupes"

@property
def profile_config(self):
return {
"test": {
"outputs": {
"dev": {
"type": "postgres",
"threads": 1,
"host": self.database_host,
"port": 5432,
"user": "root",
"pass": "password",
"dbname": "dbt",
"schema": self.unique_schema()
},
},
"target": "dev"
}
}

@use_profile("postgres")
def test_postgres_duplicate_model_enabled(self):
def test_postgres_duplicate_source_enabled(self):
message = "dbt found two resources with the name"
try:
self.run_dbt(["compile"])
self.assertTrue(False, "dbt did not throw for duplicate sources")
except CompilationException as e:
self.assertTrue(message in str(e), "dbt did not throw the correct error message")
self.assertTrue(message in str(
e), "dbt did not throw the correct error message")
6 changes: 0 additions & 6 deletions test/unit/test_contracts_graph_unparsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,6 @@ class TestUnparsedExposure(ContractTestCase):

def get_ok_dict(self):
return {
'yaml_key': 'exposures',
'name': 'my_exposure',
'type': 'dashboard',
'owner': {
Expand All @@ -592,22 +591,17 @@ def get_ok_dict(self):
'ref("my_model")',
'source("raw", "source_table")',
],
'original_file_path': '/some/fake/path',
'package_name': 'test'
}

def test_ok(self):
exposure = self.ContractType(
yaml_key='exposures',
name='my_exposure',
type=ExposureType.Dashboard,
owner=ExposureOwner(email='name@example.com'),
maturity=MaturityType.Medium,
url='https://example.com/dashboards/1',
description='A exposure',
depends_on=['ref("my_model")', 'source("raw", "source_table")'],
original_file_path='/some/fake/path',
package_name='test'
)
dct = self.get_ok_dict()
self.assert_symmetric(exposure, dct)
Expand Down