-
Notifications
You must be signed in to change notification settings - Fork 243
Feat!: Dev-only VDE mode #5087
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
Feat!: Dev-only VDE mode #5087
Changes from 20 commits
6161f36
b14b33b
19b9a31
281e3bd
854b732
8144762
2d836a0
8be9fd5
01e6876
273cb34
2c25c9d
52eb476
99c8b0b
325bf76
096f139
224cff5
1d7c7de
2ac5969
24e10dd
9e36f95
dfa0e35
06e2541
8fe9b65
a90932d
08d80bf
2670119
282cfe3
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 |
---|---|---|
|
@@ -49,6 +49,35 @@ def __repr__(self) -> str: | |
return str(self) | ||
|
||
|
||
class VirtualEnvironmentMode(str, Enum): | ||
"""Mode for virtual environment behavior. | ||
|
||
FULL: Use full virtual environment functionality with versioned table names and virtual layer updates. | ||
DEV_ONLY: Bypass virtual environments in production, using original unversioned model names. | ||
""" | ||
|
||
FULL = "full" | ||
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. Minor: Is the term "FULL" being used because of "FULL" in auto-categorization? "ALL" feels better to me but I'm wondering if there is some intention behind using "FULL" that I am missing. |
||
DEV_ONLY = "dev_only" | ||
|
||
@property | ||
def is_full(self) -> bool: | ||
return self == VirtualEnvironmentMode.FULL | ||
|
||
@property | ||
def is_dev_only(self) -> bool: | ||
return self == VirtualEnvironmentMode.DEV_ONLY | ||
|
||
@classproperty | ||
def default(cls) -> VirtualEnvironmentMode: | ||
return VirtualEnvironmentMode.FULL | ||
|
||
def __str__(self) -> str: | ||
return self.name | ||
|
||
def __repr__(self) -> str: | ||
return str(self) | ||
|
||
|
||
class TableNamingConvention(str, Enum): | ||
# Causes table names at the physical layer to follow the convention: | ||
# <schema-name>__<table-name>__<fingerprint> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,11 @@ | |
from sqlmesh.cicd.config import CICDBotConfig | ||
from sqlmesh.core import constants as c | ||
from sqlmesh.core.console import get_console | ||
from sqlmesh.core.config import EnvironmentSuffixTarget, TableNamingConvention | ||
from sqlmesh.core.config.common import ( | ||
EnvironmentSuffixTarget, | ||
TableNamingConvention, | ||
VirtualEnvironmentMode, | ||
) | ||
from sqlmesh.core.config.base import BaseConfig, UpdateStrategy | ||
from sqlmesh.core.config.common import variables_validator, compile_regex_mapping | ||
from sqlmesh.core.config.connection import ( | ||
|
@@ -110,6 +114,7 @@ class Config(BaseConfig): | |
physical_schema_mapping: A mapping from regular expressions to names of schemas in which physical tables for corresponding models will be placed. | ||
environment_suffix_target: Indicates whether to append the environment name to the schema or table name. | ||
physical_table_naming_convention: Indicates how tables should be named at the physical layer | ||
virtual_environment_mode: Indicates how environments should be handled. | ||
gateway_managed_virtual_layer: Whether the models' views in the virtual layer are created by the model-specific gateway rather than the default gateway. | ||
infer_python_dependencies: Whether to statically analyze Python code to automatically infer Python package requirements. | ||
environment_catalog_mapping: A mapping from regular expressions to catalog names. The catalog name is used to determine the target catalog for a given environment. | ||
|
@@ -148,12 +153,9 @@ class Config(BaseConfig): | |
env_vars: t.Dict[str, str] = {} | ||
username: str = "" | ||
physical_schema_mapping: RegexKeyDict = {} | ||
environment_suffix_target: EnvironmentSuffixTarget = Field( | ||
default=EnvironmentSuffixTarget.default | ||
) | ||
physical_table_naming_convention: TableNamingConvention = Field( | ||
default=TableNamingConvention.default | ||
) | ||
environment_suffix_target: EnvironmentSuffixTarget = EnvironmentSuffixTarget.default | ||
physical_table_naming_convention: TableNamingConvention = TableNamingConvention.default | ||
virtual_environment_mode: VirtualEnvironmentMode = VirtualEnvironmentMode.default | ||
gateway_managed_virtual_layer: bool = False | ||
infer_python_dependencies: bool = True | ||
environment_catalog_mapping: RegexKeyDict = {} | ||
|
@@ -260,6 +262,11 @@ def _normalize_identifiers(key: str) -> None: | |
"Please specify one or the other" | ||
) | ||
|
||
if self.plan.use_finalized_state and not self.virtual_environment_mode.is_full: | ||
raise ConfigError( | ||
"Using the finalized state is only supported when `virtual_environment_mode` is set to `full`." | ||
) | ||
Comment on lines
+265
to
+268
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. Is this because a "partial" state doesn't make sense if you aren't using VDE in prod? If so, what would be the negative in allowing this? Just doesn't have much meaning? 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. Actually, since dev-only VDE always assumes a forward-only plan, this check is no longer relevant. Will remove it, thanks 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. I thought about this more and concluded that indeed this configuration is meaningless when used with dev-only VDEs since there are no previous table versions to go back to. |
||
|
||
if self.environment_catalog_mapping: | ||
_normalize_identifiers("environment_catalog_mapping") | ||
if self.physical_schema_mapping: | ||
|
Uh oh!
There was an error while loading. Please reload this page.