Add streamlit entities #1934

sfc-gh-jsikorski · 2024-12-09T09:52:18Z

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

Added streamlit entity outline for introducing composability in PRDF v2

…ntities # Conflicts: # tests/streamlit/__snapshots__/test_commands.ambr # tests/test_data/projects/example_streamlit_v2/snowflake.yml

tests/test_data/projects/example_streamlit_v2/utils/utils.py

src/snowflake/cli/_plugins/streamlit/streamlit_entity.py

tests/streamlit/__snapshots__/test_commands.ambr

sfc-gh-gbloom · 2024-12-11T16:42:32Z

src/snowflake/cli/_plugins/streamlit/streamlit_entity.py

-from snowflake.cli.api.entities.common import EntityBase
+from snowflake.cli._plugins.workspace.context import ActionContext
+from snowflake.cli.api.entities.common import EntityBase, get_sql_executor
+from snowflake.connector.cursor import SnowflakeCursor


 class StreamlitEntity(EntityBase[StreamlitEntityModel]):


I added a temporary mock implementation of StreamlitEntity for composability with Native Apps in #1856.
The fake logic will be replaced by your PR, but please take a look at the proposed ApplicationPackageChildInterface contract.

Renamed some methods to allign it with your interface.
Bundle and deploy actions are left intact, as they will have to be heavily remodeled, after introducing project-wide bundle map and other changes to output files management

…ntities # Conflicts: # src/snowflake/cli/_plugins/streamlit/streamlit_entity.py

sfc-gh-gbloom · 2024-12-13T18:57:22Z

src/snowflake/cli/_plugins/nativeapp/entities/application_package.py

@@ -833,6 +833,7 @@ def _bundle_children(self, action_ctx: ActionContext) -> List[str]:
                child_entity.get_deploy_sql(
                    artifacts_dir=child_artifacts_dir.relative_to(self.deploy_root),
                    schema=child_schema,
+                    replace=True,


Will this be relevant for other entity types, or only Streamlit?
Should the user be able to control it?

For snowpark entities- for shure. We want user to be able to ensure, if the deploy operation is allowed to overwrite any data.

We should probably pass the replace value as it exists on the base entity, and then allow to override it for a specific child when used in an app. How will users control replace in a standalone Streamlit entity?

It doesn't have to block this PR though. Just please add a TODO comment here to address this issue, since it will now be True for all app children without being able to control it.

You got me a bit lost here. I would assume, that users will control replace just like they do now- by passing replace flag to command/action.
Assuming by the expected query in your test, it was done implicitly, without user control, as CREATE OR REPLACE was a default value.

A --replace flag works when we're deploying a single entity. But here we will have multiple child entities deployed as part of the native app, and the user might want to replace only some of them.
For now we can keep replace=True to be the implied behavior, but eventually we need to enable a way to override it.

Oh right, so, we should think of- in your PoC, or while implementing depends_on of a way, in which user can specify which mode should be used for each dependency/child

Yep. We will figure out the exact syntax later on. I was thinking about something like this:

entities: pkg: type: application package children: - target: my_sproc replace: false

sfc-gh-gbloom · 2024-12-16T16:48:33Z

src/snowflake/cli/_plugins/nativeapp/entities/application_package.py

@@ -833,6 +833,7 @@ def _bundle_children(self, action_ctx: ActionContext) -> List[str]:
                child_entity.get_deploy_sql(
                    artifacts_dir=child_artifacts_dir.relative_to(self.deploy_root),
                    schema=child_schema,
+                    replace=True,


We should probably pass the replace value as it exists on the base entity, and then allow to override it for a specific child when used in an app. How will users control replace in a standalone Streamlit entity?

It doesn't have to block this PR though. Just please add a TODO comment here to address this issue, since it will now be True for all app children without being able to control it.

src/snowflake/cli/_plugins/streamlit/streamlit_entity.py

sfc-gh-pczajka · 2024-12-17T14:51:38Z

src/snowflake/cli/_plugins/streamlit/streamlit_entity.py

+    def action_deploy(self, action_ctx: ActionContext, *args, **kwargs):
+        # After adding bundle map- we should use it's mapping here
+
+        query = self.get_deploy_sql()
+        result = self._sql_executor.execute_query(query)
+        return result


Suggested change

def action_deploy(self, action_ctx: ActionContext, *args, **kwargs):

# After adding bundle map- we should use it's mapping here

query = self.get_deploy_sql()

result = self._sql_executor.execute_query(query)

return result

def action_deploy(self, action_ctx: ActionContext, *args, **kwargs):

# After adding bundle map- we should use it's mapping here

return self._sql_executor.execute_query(self.get_deploy_sql())

To be consistent with the rest of the functions :)

sfc-gh-pczajka · 2024-12-17T14:52:58Z

src/snowflake/cli/_plugins/streamlit/streamlit_entity.py

-                PathMapping(src=str(artifact))
-                for artifact in self._entity_model.artifacts
-            ],
+    def action_deploy(self, action_ctx: ActionContext, *args, **kwargs):


Should deploy action also copy the artifacts to the stage (after resolving --replace conflicts)?

(or could you add a comment that this will be resolved in bundle map PR if thats a case)?

src/snowflake/cli/_plugins/streamlit/streamlit_entity.py

tests/streamlit/test_streamlit_entity.py

Co-authored-by: Patryk Czajka <[email protected]>

sfc-gh-jsikorski force-pushed the jsikorski/streamlit_entities branch from 015c955 to 3d36227 Compare December 11, 2024 09:45

sfc-gh-jsikorski marked this pull request as ready for review December 11, 2024 09:52

sfc-gh-jsikorski requested review from a team as code owners December 11, 2024 09:52

Actions

6bdb1af

sfc-gh-jsikorski force-pushed the jsikorski/streamlit_entities branch from 33999ec to 6bdb1af Compare December 11, 2024 11:50

Merge remote-tracking branch 'origin/main' into jsikorski/streamlit_e…

3d1a0d5

…ntities # Conflicts: # tests/streamlit/__snapshots__/test_commands.ambr # tests/test_data/projects/example_streamlit_v2/snowflake.yml

sfc-gh-pczajka reviewed Dec 11, 2024

View reviewed changes

sfc-gh-gbloom reviewed Dec 11, 2024

View reviewed changes

fix

6ccfdc8

sfc-gh-jsikorski force-pushed the jsikorski/streamlit_entities branch from 255ec61 to 6ccfdc8 Compare December 12, 2024 13:32

sfc-gh-jsikorski requested review from sfc-gh-pczajka and sfc-gh-gbloom December 12, 2024 13:34

sfc-gh-jsikorski added 2 commits December 12, 2024 14:43

text fix

a02d7c5

Merge remote-tracking branch 'origin/main' into jsikorski/streamlit_e…

f5b1a54

…ntities # Conflicts: # src/snowflake/cli/_plugins/streamlit/streamlit_entity.py

sfc-gh-jsikorski force-pushed the jsikorski/streamlit_entities branch from 07ccf39 to c6fe394 Compare December 12, 2024 19:57

sfc-gh-gbloom reviewed Dec 13, 2024

View reviewed changes

sfc-gh-jsikorski force-pushed the jsikorski/streamlit_entities branch 2 times, most recently from 9de1ca8 to 83af4d1 Compare December 16, 2024 13:22

Fixes

46f4447

sfc-gh-jsikorski force-pushed the jsikorski/streamlit_entities branch from 83af4d1 to 46f4447 Compare December 16, 2024 14:10

sfc-gh-jsikorski requested review from sfc-gh-gbloom and a team December 16, 2024 14:25

sfc-gh-gbloom previously approved these changes Dec 16, 2024

View reviewed changes

sfc-gh-pczajka reviewed Dec 17, 2024

View reviewed changes

src/snowflake/cli/_plugins/streamlit/streamlit_entity.py Outdated Show resolved Hide resolved

sfc-gh-pczajka reviewed Dec 17, 2024

View reviewed changes

tests/streamlit/test_streamlit_entity.py Outdated Show resolved Hide resolved

sfc-gh-jsikorski dismissed sfc-gh-gbloom’s stale review via be11eed December 17, 2024 15:42

Fix

423eff7

Co-authored-by: Patryk Czajka <[email protected]>

sfc-gh-jsikorski force-pushed the jsikorski/streamlit_entities branch from cb2d9fe to 423eff7 Compare December 17, 2024 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add streamlit entities #1934

Add streamlit entities #1934

sfc-gh-jsikorski commented Dec 9, 2024 •

edited

Loading

sfc-gh-gbloom Dec 11, 2024

sfc-gh-jsikorski Dec 12, 2024

sfc-gh-gbloom Dec 13, 2024

sfc-gh-jsikorski Dec 16, 2024

sfc-gh-gbloom Dec 16, 2024

sfc-gh-jsikorski Dec 17, 2024

sfc-gh-gbloom Dec 17, 2024

sfc-gh-jsikorski Dec 17, 2024

sfc-gh-gbloom Dec 17, 2024

sfc-gh-gbloom Dec 16, 2024

sfc-gh-pczajka Dec 17, 2024

sfc-gh-pczajka Dec 17, 2024

sfc-gh-pczajka Dec 17, 2024

sfc-gh-jsikorski Dec 17, 2024

Add streamlit entities #1934

Are you sure you want to change the base?

Add streamlit entities #1934

Conversation

sfc-gh-jsikorski commented Dec 9, 2024 • edited Loading

Pre-review checklist

Changes description

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-jsikorski commented Dec 9, 2024 •

edited

Loading