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

Project definition v2 entity schemas: application and application package #1280

Merged
merged 33 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0eaf420
application and application package entity schemas
sfc-gh-gbloom Jul 8, 2024
f7dc71f
switch internal entity type field names
sfc-gh-gbloom Jul 8, 2024
1e806be
add integration test
sfc-gh-gbloom Jul 8, 2024
65d62f5
fix rebase
sfc-gh-gbloom Jul 8, 2024
8126435
post deploy hook type
sfc-gh-gbloom Jul 8, 2024
1858673
use env vars
sfc-gh-gbloom Jul 8, 2024
244483a
rename test name
sfc-gh-gbloom Jul 8, 2024
984a046
entity type enum
sfc-gh-gbloom Jul 8, 2024
240fb53
simplify feature flag
sfc-gh-gbloom Jul 9, 2024
d618d8f
types
sfc-gh-gbloom Jul 9, 2024
cb7b090
Merge branch 'main' into pdfv2-app-package-entity
sfc-gh-gbloom Jul 9, 2024
35617a7
add app package fields
sfc-gh-gbloom Jul 9, 2024
df209b3
Merge branch 'main' into pdfv2-app-package-entity
sfc-gh-gbloom Jul 9, 2024
2b85a04
simplify entity type, use discriminator
sfc-gh-gbloom Jul 9, 2024
4c84b9d
test model type
sfc-gh-gbloom Jul 9, 2024
c66479e
remove schema alias
sfc-gh-gbloom Jul 9, 2024
2c9793d
remove "noqa A003" comments
sfc-gh-gbloom Jul 9, 2024
fd475e9
revert schema alias
sfc-gh-gbloom Jul 9, 2024
1fd8975
revert noqa A003
sfc-gh-gbloom Jul 10, 2024
79f06fb
Merge branch 'main' into pdfv2-app-package-entity
sfc-gh-gbloom Jul 10, 2024
56b9847
revert noqa A003
sfc-gh-gbloom Jul 10, 2024
bd79cd0
update types
sfc-gh-gbloom Jul 10, 2024
a6c6ab2
id fields
sfc-gh-gbloom Jul 10, 2024
90e6f2c
create types map dynamically
sfc-gh-gbloom Jul 11, 2024
5227eda
get_type method
sfc-gh-gbloom Jul 11, 2024
d5188dc
Merge branch 'main' into pdfv2-app-package-entity
sfc-gh-gbloom Jul 11, 2024
6b3faa6
revert entity type
sfc-gh-gbloom Jul 11, 2024
c25b4fb
read entity list from union
sfc-gh-gbloom Jul 11, 2024
1beb393
validate target field method
sfc-gh-gbloom Jul 11, 2024
f4fa56a
Merge branch 'main' into pdfv2-app-package-entity
sfc-gh-gbloom Jul 11, 2024
c49d6d6
Merge branch 'main' into pdfv2-app-package-entity
sfc-gh-gbloom Jul 12, 2024
e0555b0
Merge branch 'main' into pdfv2-app-package-entity
sfc-gh-gbloom Jul 12, 2024
9665a6e
Merge branch 'main' into pdfv2-app-package-entity
sfc-gh-gbloom Jul 12, 2024
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 pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ select = [
"FA100", # Missing from __future__ import annotations
"W605", # Invalid escape sequences
]
ignore = [
"A003" # Class attribute shadowing a Python builtin
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ignoring flake8 A003 and removing some noqa: A003 comments. This prevents us from adding a type field on Entity classes. Using an alias prevents us from using discriminators (not supported by Pydantic).

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like it, it's a known source of errors. It's not also limited to type but also other builtins.

I think noqa: A003 on single line in model (well, every model) is safer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's fair. I tried to avoid having noqa on each entity type. Reverted.


[tool.pytest.ini_options]
addopts = "-vv --maxfail=10 -m 'not integration and not performance and not e2e and not spcs and not loaded_modules and not integration_experimental'"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Copyright (c) 2024 Snowflake Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

from typing import Literal, Optional

from pydantic import AliasChoices, Field
from snowflake.cli.api.project.schemas.entities.application_package_entity import (
ApplicationPackageEntity,
)
from snowflake.cli.api.project.schemas.entities.common import (
EntityBase,
TargetField,
)
from snowflake.cli.api.project.schemas.updatable_model import (
UpdatableModel,
)


class ApplicationEntity(EntityBase):
type: Literal["application"]
name: str = Field(
title="Name of the application created when this entity is deployed"
)
from_: ApplicationFromField = Field(
validation_alias=AliasChoices("from"),
title="An application package this entity should be created from",
)
debug: Optional[bool] = Field(
title="Whether to enable debug mode when using a named stage to create an application object",
default=None,
)


class ApplicationFromField(UpdatableModel):
Copy link
Contributor

@sfc-gh-cgorrie sfc-gh-cgorrie Jul 8, 2024

Choose a reason for hiding this comment

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

(Non-blocking) In the future, we should pull the generic type attr up, e.g.

from_: EntityReferenceField[ApplicationPackageEntity]

target: TargetField[ApplicationPackageEntity] = Field(
title="Reference to an application package entity",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Copyright (c) 2024 Snowflake Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

from typing import List, Literal, Optional, Union

from pydantic import Field
from snowflake.cli.api.project.schemas.entities.common import (
EntityBase,
)
from snowflake.cli.api.project.schemas.native_app.package import DistributionOptions
from snowflake.cli.api.project.schemas.native_app.path_mapping import PathMapping


class ApplicationPackageEntity(EntityBase):
Copy link
Contributor

Choose a reason for hiding this comment

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

There are some properties not accounted for here, like scratch_stage, generated_root, and bundle_root. What's the plan for those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added these as well. While we maintain both versions we will have to remember to update v2 schema when we make v1 changes.

type: Literal["application package"]
name: str = Field(
title="Name of the application package created when this entity is deployed"
)
artifacts: List[Union[PathMapping, str]] = Field(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why union? Won't Path be enough? Using str causes problems when testing across different OS.

Copy link
Contributor

Choose a reason for hiding this comment

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

bundle_root, deploy_root, generated_root should be Path too? Will defaults work on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated all of these to Path.
The models here basically mirror the existing NA schema. I tried to align with the existing types to make it easier to work with the existing code, but we will gradually improve these when we start refactoring the underlying code (probably when we switch to v1->v2 conversion).
For v1 we have additional path validations and transformations downstream, but I agree that it's better to validate the fields earlier here if possible.

title="List of file source and destination pairs to add to the deploy root",
)
bundle_root: Optional[str] = Field(
title="Folder at the root of your project where artifacts necessary to perform the bundle step are stored.",
default="output/bundle/",
)
deploy_root: Optional[str] = Field(
title="Folder at the root of your project where the build step copies the artifacts",
default="output/deploy/",
)
generated_root: Optional[str] = Field(
title="Subdirectory of the deploy root where files generated by the Snowflake CLI will be written.",
default="__generated/",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should default be a Path or conversion happens automagicaly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Automagically, but made it Path here too to make it clearer

)
stage: Optional[str] = Field(
title="Identifier of the stage that stores the application artifacts.",
default="app_src.stage",
)
Copy link
Contributor

@sfc-gh-turbaszek sfc-gh-turbaszek Jul 10, 2024

Choose a reason for hiding this comment

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

Should we consider introducing IdentifierField that would run validation that the passed value is proper SQL identifier? What's more it may be useful to cast it to FQN instance for further reference.

I think same can be applied to name and any other field being a SQL identifier. Later when we add support for schema and database we can use model validators to validate that for example there's no FQN name passed together with database and schema.

Edit: It seems we do have IdentifierField why not use it consistently?

scratch_stage: Optional[str] = Field(
title="Identifier of the stage that stores temporary scratch data used by the Snowflake CLI.",
default="app_src.stage_snowflake_cli_scratch",
)
distribution: Optional[DistributionOptions] = Field(
title="Distribution of the application package created by the Snowflake CLI",
default="internal",
)
manifest: str = Field(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
manifest: str = Field(
manifest: Path = Field(

Or SecurePath.

title="Path to manifest.yml",
)
87 changes: 87 additions & 0 deletions src/snowflake/cli/api/project/schemas/entities/common.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Copyright (c) 2024 Snowflake Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

from abc import ABC
from enum import Enum
from typing import Generic, List, Optional, TypeVar

from pydantic import AliasChoices, Field, GetCoreSchemaHandler, ValidationInfo
from pydantic_core import core_schema
from snowflake.cli.api.project.schemas.native_app.application import (
ApplicationPostDeployHook,
)
from snowflake.cli.api.project.schemas.updatable_model import (
IdentifierField,
UpdatableModel,
)


class EntityType(Enum):
APPLICATION = "application"
APPLICATION_PACKAGE = "application package"
Copy link
Contributor

Choose a reason for hiding this comment

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

We have those strings as literal on entities classes and we have them here. Should we consider using this Enum as source of truth? Or should we use entities to build Enum / something?

Taking into account that we are constructing Union[ApplicationEntity, ApplicationPackageEntity] and

_v2_entity_types_map = {
    EntityType.APPLICATION.value: ApplicationEntity,
    EntityType.APPLICATION_PACKAGE.value: ApplicationPackageEntity,
}

It means that adding new entity requires it to be registered in at least two places.

it may be wise to have

KNOWN_ENTITIES = [
  ApplicationEntity,
  ApplicationPackageEntity
]

ENTITIES_TYPE = Union[*KNOWN_ENTITIES]
_v2_entity_types_map = {get_args(eval(inspect.get_annotations(e)["type"]))[0]: e for e in KNOWN_ENTITIES}

The dict comprehension is a bit of a magic, but probably there's something smarter we can do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's what I tried at first but I couldn't make pydantic understand enums as discriminators and reverted to literal strings.

I'm not sure what I think about this dict comprehension, but I get the idea. I'll try to come up with a cleaner way to have a single source of truth.



class MetaField(UpdatableModel):
warehouse: Optional[str] = IdentifierField(
title="Warehouse used to run the scripts", default=None
)
role: Optional[str] = IdentifierField(
title="Role to use when creating the entity object",
default=None,
)
post_deploy: Optional[List[ApplicationPostDeployHook]] = Field(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this common for all entities?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep

title="Actions that will be executed after the application object is created/upgraded",
default=None,
)


class DefaultsField(UpdatableModel):
schema_: Optional[str] = Field(
title="Schema.",
validation_alias=AliasChoices("schema"),
default=None,
)
stage: Optional[str] = Field(
title="Stage.",
default=None,
)


class EntityBase(ABC, UpdatableModel):
Copy link
Contributor

Choose a reason for hiding this comment

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

(Non-blocking) I would like to see e.g. StageBasedEntity, DatabaseLevelEntity, SchemaLevelEntity, AccountLevelEntity in the future to maximize re-use and align with the func spec

meta: Optional[MetaField] = Field(title="Meta fields", default=None)


TargetType = TypeVar("TargetType")


class TargetField(Generic[TargetType]):
def __init__(self, entity_target_key: str):
self.value = entity_target_key

def __repr__(self):
return self.value

@classmethod
def validate(cls, value: str, info: ValidationInfo) -> TargetField:
return cls(value)

@classmethod
def __get_pydantic_core_schema__(
cls, source_type, handler: GetCoreSchemaHandler
) -> core_schema.CoreSchema:
return core_schema.with_info_after_validator_function(
cls.validate, handler(str), field_name=handler.field_name
)
26 changes: 26 additions & 0 deletions src/snowflake/cli/api/project/schemas/entities/entities.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Copyright (c) 2024 Snowflake Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from __future__ import annotations

from typing import Union

from snowflake.cli.api.project.schemas.entities.application_entity import (
ApplicationEntity,
)
from snowflake.cli.api.project.schemas.entities.application_package_entity import (
ApplicationPackageEntity,
)

Entity = Union[ApplicationEntity, ApplicationPackageEntity]
79 changes: 70 additions & 9 deletions src/snowflake/cli/api/project/schemas/project_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,35 @@
from typing import Dict, Optional, Union

from packaging.version import Version
from pydantic import Field, ValidationError, field_validator
from pydantic import Field, ValidationError, field_validator, model_validator
from snowflake.cli.api.feature_flags import FeatureFlag
from snowflake.cli.api.project.errors import SchemaValidationError
from snowflake.cli.api.project.schemas.entities.application_entity import (
ApplicationEntity,
)
from snowflake.cli.api.project.schemas.entities.application_package_entity import (
ApplicationPackageEntity,
)
from snowflake.cli.api.project.schemas.entities.common import (
DefaultsField,
EntityType,
TargetField,
)
from snowflake.cli.api.project.schemas.entities.entities import (
Entity,
)
from snowflake.cli.api.project.schemas.native_app.native_app import NativeApp
from snowflake.cli.api.project.schemas.snowpark.snowpark import Snowpark
from snowflake.cli.api.project.schemas.streamlit.streamlit import Streamlit
from snowflake.cli.api.project.schemas.updatable_model import UpdatableModel
from snowflake.cli.api.utils.models import ProjectEnvironment
from snowflake.cli.api.utils.types import Context
from typing_extensions import Annotated

_v2_entity_types_map = {
EntityType.APPLICATION.value: ApplicationEntity,
EntityType.APPLICATION_PACKAGE.value: ApplicationPackageEntity,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Next up for us is to figure out how these pydantic models map to classes with e.g. deploy, drop, bundle methods



@dataclass
Expand Down Expand Up @@ -105,8 +125,55 @@ def _convert_env(


class DefinitionV20(_ProjectDefinitionBase):
entities: Dict = Field(
title="Entity definitions.",
entities: Dict[str, Annotated[Entity, Field(discriminator="type")]] = Field(
title="Entity definitions."
)

@model_validator(mode="before")
@classmethod
def apply_defaults(cls, data: Dict) -> Dict:
"""
Applies default values that exist on the model but not specified in yml
"""
if "defaults" in data:
if "entities" in data:
Copy link
Contributor

@sfc-gh-astus sfc-gh-astus Jul 10, 2024

Choose a reason for hiding this comment

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

if "defaults" in data and "entities" in data:

for key, entity in data["entities"].items():
entity_type = entity["type"]
if entity_type not in _v2_entity_types_map:
continue
entity_model = _v2_entity_types_map[entity_type]
for default_key, default_value in data["defaults"].items():
if (
default_key in entity_model.model_fields
and default_key not in entity
):
entity[default_key] = default_value
return data

@field_validator("entities", mode="after")
@classmethod
def validate_entities(cls, entities: Dict[str, Entity]) -> Dict[str, Entity]:
for key, entity in entities.items():
# TODO Automatically detect TargetFields to validate
if entity.type == EntityType.APPLICATION.value:
if isinstance(entity.from_.target, TargetField):
target = str(entity.from_.target)
if target not in entities:
raise ValueError(f"No such target: {target}")
else:
# Validate the target type
target_cls = entity.from_.__class__.model_fields["target"]
target_type = target_cls.annotation.__args__[0]
actual_target_type = entities[target].__class__
if target_type and target_type is not actual_target_type:
raise ValueError(
f"Target type mismatch. Expected {target_type.__name__}, got {actual_target_type.__name__}"
)
return entities

defaults: Optional[DefaultsField] = Field(
title="Default key/value entity values that are merged recursively for each entity.",
default=None,
)

env: Union[Dict[str, str], ProjectEnvironment, None] = Field(
Expand All @@ -125,12 +192,6 @@ def _convert_env(
return env
return ProjectEnvironment(default_env=(env or {}), override_env={})

@field_validator("entities")
@classmethod
def validate_entities(cls, entities: Dict) -> Dict:
# TODO Add entities validation logic
return entities


def build_project_definition(**data):
"""
Expand Down
2 changes: 1 addition & 1 deletion src/snowflake/cli/api/secure_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def write_text(self, *args, **kwargs):
self.path.write_text(*args, **kwargs)

@contextmanager
def open( # noqa: A003
def open(
self,
mode="r",
read_file_limit_mb: Optional[int] = None,
Expand Down
2 changes: 1 addition & 1 deletion src/snowflake/cli/api/utils/rendering.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class IgnoreAttrEnvironment(Environment):
Only dict items can be used for rendering.
"""

def getattr(self, obj, attribute): # noqa: A003
def getattr(self, obj, attribute):
try:
return obj[attribute]
except (TypeError, LookupError, AttributeError):
Expand Down
2 changes: 1 addition & 1 deletion src/snowflake/cli/app/commands_registration/threadsafe.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def __init__(self, value: T):
self._value = value
self._lock = threading.Lock()

def set(self, new_value: T) -> T: # noqa: A003
def set(self, new_value: T) -> T:
return self.transform(lambda _: new_value)

def transform(self, f: Callable[[T], T]) -> T:
Expand Down
Loading
Loading