Snow 1761248 introduce notebook entity #2007

sfc-gh-pczajka · 2025-01-20T17:02:34Z

Pre-review checklist

I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
I've added or updated automated unit tests to verify correctness of my new code.
I've added or updated integration tests to verify correctness of my new code.
I've confirmed that my changes are working by executing CLI's commands manually on MacOS.
I've confirmed that my changes are working by executing CLI's commands manually on Windows.
I've confirmed that my changes are up-to-date with the target branch.
I've described my changes in the release notes.
I've described my changes in the section below.

Changes description

Introduce notebook entity and add command snow notebook deploy based on entities

sfc-gh-jsikorski · 2025-01-20T17:52:30Z

src/snowflake/cli/_plugins/notebook/notebook_entity.py

+
+    @functools.cached_property
+    def _connection(self) -> SnowflakeConnection:
+        return get_cli_context().connection


These two overwrite properties from parent class without any change. Are they needed?

Nice catch!
I was basing on streamlit_entity implementation, removed

sfc-gh-jsikorski · 2025-01-20T17:53:15Z

src/snowflake/cli/_plugins/notebook/notebook_entity.py

+        try:
+            self._sql_executor.execute_query(
+                f"DESCRIBE NOTEBOOK {self._fqn.sql_identifier}"
+            )


The SQL here could be extracted to get_describe_sql method - would be useful, when adding more actions
Also, maybe action_describe is the way to go?

sfc-gh-jsikorski · 2025-01-20T17:53:55Z

src/snowflake/cli/_plugins/notebook/notebook_entity.py

+        return self._sql_executor.execute_query(queries)
+
+    def action_deploy(
+        self,


If we have action_deploy - we probably should have an action_drop

It currently won't be used by any command - should we implement it in this PR?

If we introduce an entity- i think we should at least make some dummy methods to show, what is left to implement

I've added placeholders

sfc-gh-jsikorski · 2025-01-20T17:57:19Z

src/snowflake/cli/_plugins/notebook/notebook_entity.py

+        stage_path = self._notebook_file_stage_path
+        cli_console.step("Creating notebook")
+        create_str = "CREATE OR REPLACE" if replace else "CREATE"
+        queries = dedent(


Please extract to get_create_sql - will be useful for creating more actions and if notebooks will be used as native apps children

sfc-gh-jsikorski · 2025-01-21T09:47:56Z

src/snowflake/cli/_plugins/notebook/notebook_entity.py

+        return get_cli_context().connection
+
+    @functools.cached_property
+    def _fqn(self) -> FQN:


You can use self.fqn as well

sfc-gh-jsikorski · 2025-01-21T10:01:41Z

src/snowflake/cli/_plugins/notebook/notebook_entity.py

+
+class NotebookEntity(EntityBase[NotebookEntityModel]):
+    """
+    A notebook.


Indeed 😺

RELEASE-NOTES.md

sfc-gh-turbaszek · 2025-01-23T14:37:38Z

src/snowflake/cli/_plugins/notebook/commands.py

+    replace: bool = ReplaceOption(
+        help="Replace existing Notebook if it already exists.",
+    ),
+    if_not_exists: bool = IfNotExistsOption(help="Skip if Notebook already exists."),


Current approach was to fail if object exists. Do we plan to add this flag to other commands as well?

For some reason I was sure I've seen that in streamlit; removed

src/snowflake/cli/_plugins/notebook/commands.py

sfc-gh-turbaszek · 2025-01-23T14:39:37Z

src/snowflake/cli/_plugins/notebook/commands.py

+    **options,
+) -> CommandResult:
+    """Uploads a notebook to a stage and creates a notebook. If entity_id is not provided,
+    deploys all notebooks defined in the project definition."""


Suggested change

deploys all notebooks defined in the project definition."""

deploys all notebooks defined in the project definition."""

See how streamlit works. Also consider borrowing this helper:

snowflake-cli/src/snowflake/cli/_plugins/streamlit/commands.py

Lines 158 to 163 in 347c46a

streamlit: StreamlitEntityModel = get_entity_for_operation(

cli_context=cli_context,

entity_id=entity_id,

project_definition=pd,

entity_type="streamlit",

)

Fixed, nice helper!

Co-authored-by: Tomasz Urbaszek <[email protected]>

sfc-gh-turbaszek · 2025-01-27T15:03:25Z

src/snowflake/cli/_plugins/notebook/notebook_entity_model.py

+    stage_path: Optional[str] = Field(
+        title="Stage directory in which the notebook file will be stored", default=None
+    )
+    notebook_file: Path = Field(title="Notebook file")


Should we consider using artifact approach with notbeook_file being a pointer (similar to streamlit main file)? In this way you can upload more than one file, for example .py file with scripts or a .csv data. WDYT?

Good idea, added

sfc-gh-turbaszek · 2025-01-29T15:22:23Z

src/snowflake/cli/_plugins/notebook/notebook_entity.py

+            return False
+
+    def _upload_artifacts(self):
+        stage_fqn = FQN.from_stage(self._stage_path.stage)


Don't we have _stage.path.get_fqn()? If not wdyt about adding it?

sfc-gh-turbaszek · 2025-01-29T15:23:17Z

src/snowflake/cli/_plugins/notebook/notebook_entity.py

+        *args,
+        **kwargs,
+    ) -> str:
+        with cli_console.phase(f"Deploying notebook {self.fqn}"):


Consider splitting this into two phases:

Uploading artifacts

Creating notebook

sfc-gh-pczajka added 8 commits January 17, 2025 15:45

IN PROGRESS

5464f82

IN PROGRESS

a1e1967

IN PROGRESS

6005d36

notebook deploy

8560ac0

check project definition version

212ea38

Different stage default paths for different notebooks

8a4b172

unit tests

29437cc

integration tests

2f1b7cd

sfc-gh-pczajka requested review from a team as code owners January 20, 2025 17:02

sfc-gh-pczajka added 4 commits January 20, 2025 18:04

update release notes

78c1880

Merge branch 'main' into SNOW-1761248-introduce-notebook-ity

d5e7ef3

Add unit test

667bd9d

Merge branch 'main' into SNOW-1761248-introduce-notebook-entity

bc2c2a5

sfc-gh-jsikorski reviewed Jan 21, 2025

View reviewed changes

sfc-gh-turbaszek reviewed Jan 23, 2025

View reviewed changes

RELEASE-NOTES.md Outdated Show resolved Hide resolved

sfc-gh-turbaszek reviewed Jan 23, 2025

View reviewed changes

src/snowflake/cli/_plugins/notebook/commands.py Outdated Show resolved Hide resolved

sfc-gh-turbaszek reviewed Jan 23, 2025

View reviewed changes

sfc-gh-pczajka and others added 11 commits January 24, 2025 10:47

Update RELEASE-NOTES.md

78e7a58

Co-authored-by: Tomasz Urbaszek <[email protected]>

Update src/snowflake/cli/_plugins/notebook/commands.py

740a15b

Co-authored-by: Tomasz Urbaszek <[email protected]>

improve help messages

16b6b6c

snapshot update

2c8ec66

remove if-not-exists

d75c216

split logic into actions

4d7e437

IN PROGRESS

86f8771

integration tests

fd4a6ba

fix help message

3d7a269

unit tests

0adb41f

Add complimentary actions

a48f8f9

sfc-gh-pczajka added 2 commits January 27, 2025 15:30

Merge branch 'main' into SNOW-1761248-introduce-notebook-entity

bb665a6

update release notes

ebf8d07

sfc-gh-turbaszek reviewed Jan 27, 2025

View reviewed changes

sfc-gh-pczajka added 5 commits January 27, 2025 16:50

Windows test fix

73f2108

Add artifacts to notebook

1aed9fc

small SPCS refactor

fe9d572

fix tests

ba670ed

Merge branch 'main' into SNOW-1761248-introduce-notebook-entity

168dc39

sfc-gh-turbaszek reviewed Jan 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Snow 1761248 introduce notebook entity #2007

Snow 1761248 introduce notebook entity #2007

sfc-gh-pczajka commented Jan 20, 2025 •

edited

Loading

sfc-gh-jsikorski Jan 20, 2025

sfc-gh-pczajka Jan 24, 2025 •

edited

Loading

sfc-gh-jsikorski Jan 20, 2025

sfc-gh-pczajka Jan 24, 2025

sfc-gh-jsikorski Jan 20, 2025

sfc-gh-pczajka Jan 24, 2025

sfc-gh-jsikorski Jan 24, 2025

sfc-gh-pczajka Jan 27, 2025

sfc-gh-jsikorski Jan 20, 2025

sfc-gh-pczajka Jan 24, 2025

sfc-gh-jsikorski Jan 21, 2025

sfc-gh-pczajka Jan 24, 2025

sfc-gh-jsikorski Jan 21, 2025

sfc-gh-turbaszek Jan 23, 2025

sfc-gh-pczajka Jan 24, 2025

sfc-gh-turbaszek Jan 23, 2025

sfc-gh-pczajka Jan 27, 2025

sfc-gh-turbaszek Jan 27, 2025

sfc-gh-pczajka Jan 29, 2025

sfc-gh-turbaszek Jan 29, 2025

sfc-gh-turbaszek Jan 29, 2025

	deploys all notebooks defined in the project definition."""
	deploys all notebooks defined in the project definition."""

	streamlit: StreamlitEntityModel = get_entity_for_operation(
	cli_context=cli_context,
	entity_id=entity_id,
	project_definition=pd,
	entity_type="streamlit",
	)

Snow 1761248 introduce notebook entity #2007

Are you sure you want to change the base?

Snow 1761248 introduce notebook entity #2007

Conversation

sfc-gh-pczajka commented Jan 20, 2025 • edited Loading

Pre-review checklist

Changes description

Choose a reason for hiding this comment

sfc-gh-pczajka Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-pczajka commented Jan 20, 2025 •

edited

Loading

sfc-gh-pczajka Jan 24, 2025 •

edited

Loading