Skip to content

Conversation

@sfc-gh-zblackwood
Copy link
Contributor

No description provided.

Copy link

@tylerjrichards tylerjrichards left a comment

Choose a reason for hiding this comment

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

so sick, love it

@@ -0,0 +1,9 @@
definition_version: 1

Choose a reason for hiding this comment

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

what is this used for? should it always be 1?

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, it's always 1 currently. The idea (which is fairly common in other config files) is that if you change the spec in a non-backwards-compatible way, you can change the definition version so the user and the parser knows which one to use. I can definitely add a comment on this, though.

from common.get_data import get_events
from snowflake.snowpark.functions import count_distinct
from utils import (
from common.utils import (

Choose a reason for hiding this comment

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

love this

main_file: streamlit_app.py
env_file: environment.yml
pages_dir: pages/
additional_source_files:

Choose a reason for hiding this comment

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

maybe we can add a comment that is like "this grabs all the python files in the common folder"

@@ -242,76 +241,6 @@ def format_sql_from_df(df: DataFrame, use_header: bool = True) -> str:
return header + format_sql(str(df._plan.queries[0].sql))


@st.cache_data

Choose a reason for hiding this comment

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

I like this way better without

- snowflake
dependencies:
- python=3.8.13 # The latest python available in snowflake as of 2024-02-12
- streamlit=1.26.0 # The latest streamlit available in snowflake as of 2024-02-12

Choose a reason for hiding this comment

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

so streamlit is 1.22 in SiS for most customers right now, we should be going into a public preview next week so I think this is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good catch. Should I update the comment to reflect the date, or is this fine as-is?

Copy link
Contributor

Choose a reason for hiding this comment

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

nah it is fine as is I think

@sfc-gh-trichards
Copy link
Contributor

@iamontheinet wdyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants