-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add exports config to YAML spec for saved queries #190
Changes from all commits
69f077b
467efab
fde9dc2
eeb3dd7
929962a
8daa65c
7df2788
5c023a6
349c8ff
a05a008
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
kind: Features | ||
body: Add exports configuration to YAML spec. | ||
time: 2023-10-24T16:28:42.013032-07:00 | ||
custom: | ||
Author: courtneyholcomb | ||
Issue: "189" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
from __future__ import annotations | ||
|
||
from typing import Optional | ||
|
||
from pydantic import Field | ||
from typing_extensions import override | ||
|
||
from dbt_semantic_interfaces.implementations.base import HashableBaseModel | ||
from dbt_semantic_interfaces.protocols import ProtocolHint | ||
from dbt_semantic_interfaces.protocols.export import Export, ExportConfig | ||
from dbt_semantic_interfaces.type_enums.export_destination_type import ( | ||
ExportDestinationType, | ||
) | ||
|
||
|
||
class PydanticExportConfig(HashableBaseModel, ProtocolHint[ExportConfig]): | ||
"""Pydantic implementation of ExportConfig. | ||
|
||
Note on `schema_name`: `schema` is an existing BaseModel attribute, so we need to alias it here. | ||
`Field.alias="schema"` enables using the `schema` key in YAML. `Config.allow_population_by_field_name` | ||
enables parsing for both `schema` and `schema_name` when deserializing from JSON. | ||
""" | ||
|
||
class Config: # noqa: D | ||
allow_population_by_field_name = True | ||
|
||
@override | ||
def _implements_protocol(self) -> ExportConfig: | ||
return self | ||
|
||
export_as: ExportDestinationType | ||
schema_name: Optional[str] = Field(alias="schema", default=None) | ||
alias: Optional[str] = None | ||
|
||
|
||
class PydanticExport(HashableBaseModel, ProtocolHint[Export]): | ||
"""Pydantic implementation of Export.""" | ||
|
||
@override | ||
def _implements_protocol(self) -> Export: | ||
return self | ||
|
||
name: str | ||
config: PydanticExportConfig |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
from __future__ import annotations | ||
|
||
from abc import abstractmethod | ||
from typing import Optional, Protocol | ||
|
||
from dbt_semantic_interfaces.type_enums.export_destination_type import ( | ||
ExportDestinationType, | ||
) | ||
|
||
|
||
class Export(Protocol): | ||
"""Configuration for writing query results to a table.""" | ||
|
||
@property | ||
@abstractmethod | ||
def name(self) -> str: # noqa: D | ||
pass | ||
|
||
@property | ||
@abstractmethod | ||
def config(self) -> ExportConfig: # noqa: D | ||
pass | ||
|
||
|
||
class ExportConfig(Protocol): | ||
"""Nested configuration attributes for exports.""" | ||
|
||
@property | ||
@abstractmethod | ||
def export_as(self) -> ExportDestinationType: | ||
"""Type of destination to write export to.""" | ||
pass | ||
|
||
@property | ||
@abstractmethod | ||
def schema_name(self) -> Optional[str]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @QMalcolm the core parser can take There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is indeed the case. We have a separate unparsed and parsed node representation. It'll be |
||
"""Schema to write export to. Defaults to deployment schema.""" | ||
pass | ||
|
||
@property | ||
@abstractmethod | ||
def alias(self) -> Optional[str]: | ||
"""Name for table/filte export is written to. Defaults to export name.""" | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
from dbt_semantic_interfaces.enum_extension import ExtendedEnum | ||
|
||
|
||
class ExportDestinationType(ExtendedEnum): | ||
"""Types of destinations that exports can be written to.""" | ||
|
||
TABLE = "table" | ||
VIEW = "view" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,9 @@ | |
parse_yaml_files_to_semantic_manifest, | ||
) | ||
from dbt_semantic_interfaces.parsing.objects import YamlConfigFile | ||
from dbt_semantic_interfaces.type_enums.export_destination_type import ( | ||
ExportDestinationType, | ||
) | ||
from tests.example_project_configuration import ( | ||
EXAMPLE_PROJECT_CONFIGURATION_YAML_CONFIG_FILE, | ||
) | ||
|
@@ -134,3 +137,43 @@ def test_saved_query_where() -> None: | |
assert saved_query.where is not None | ||
assert len(saved_query.where.where_filters) == 1 | ||
assert where == saved_query.where.where_filters[0].where_sql_template | ||
|
||
|
||
def test_saved_query_exports() -> None: | ||
"""Test for parsing exports referenced in a saved query.""" | ||
yaml_contents = textwrap.dedent( | ||
"""\ | ||
saved_query: | ||
name: test_exports | ||
metrics: | ||
- test_metric_a | ||
exports: | ||
- name: test_exports1 | ||
config: | ||
export_as: VIEW | ||
schema: my_schema | ||
alias: my_view_name | ||
- name: test_exports2 | ||
config: | ||
export_as: table | ||
""" | ||
) | ||
file = YamlConfigFile(filepath="inline_for_test", contents=yaml_contents) | ||
|
||
build_result = parse_yaml_files_to_semantic_manifest(files=[file, EXAMPLE_PROJECT_CONFIGURATION_YAML_CONFIG_FILE]) | ||
|
||
assert len(build_result.semantic_manifest.saved_queries) == 1 | ||
saved_query = build_result.semantic_manifest.saved_queries[0] | ||
assert saved_query.exports and len(saved_query.exports) == 2 | ||
names_to_exports = {export.name: export for export in saved_query.exports} | ||
assert set(names_to_exports.keys()) == {"test_exports1", "test_exports2"} | ||
|
||
export1_config = names_to_exports["test_exports1"].config | ||
assert export1_config.export_as == ExportDestinationType.VIEW | ||
assert export1_config.schema_name == "my_schema" | ||
assert export1_config.alias == "my_view_name" | ||
|
||
export2_config = names_to_exports["test_exports2"].config | ||
assert export2_config.export_as == ExportDestinationType.TABLE | ||
assert export2_config.schema_name is None | ||
assert export2_config.alias is None | ||
Comment on lines
+178
to
+179
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question for the folks who designed the spec - do we want to allow this? It seems like we can't actually fulfill this contract because there's no way to tell a warehouse "here, put this data..... someplace." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note - if this will be caught by a validation PR I haven't read yet, just tell me to go away. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the table name? Or is the table name the export name? Is that obvious to people? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep it will default to the export name. I have no idea if that's obvious to people but it should be in the docs when we get to that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This make target was mentioned in CI, but it didn't actually exist in the repo so I added it.