Post_deploy script should run relative to project root, Fixes #1325. #1340
Conversation
| @@ -141,13 +145,18 @@ def _execute_sql_script(self, sql_script_path): | |||
| This assumes that a relevant warehouse is already active. | |||
| Consequently, "use database" will be executed first if it is set in definition file or in the current connection. | |||
| """ | |||
| with open(sql_script_path) as f: | |||
| sql_script = f.read() | |||
| env = get_sql_cli_jinja_env( | |||
There was a problem hiding this comment.
We exploded snowflake_sql_jinja_render, any reason we couldn't modify the existing abstraction instead?
There was a problem hiding this comment.
Also, we're diverging between how package scripts are executed and how post-deploy scripts are executed. For package scripts, the env is created differently, and AFAICT if a script fails to expand we guarantee that no script is executed. This doesn't appear to be the case here.
There was a problem hiding this comment.
I have refactored to reuse some of the existing package script logic, and also make sure that we check for template errors before executing any of the scripts.
| @@ -561,6 +561,27 @@ def create_app_package(self) -> None: | |||
| ) | |||
| ) | |||
|
|
|||
| def _expand_script_templates( | |||
| self, env: jinja2.Environment, jinja_context, scripts: List[str] | |||
| ): | |||
There was a problem hiding this comment.
document the return type of this
| def _expand_script_templates( | ||
| self, env: jinja2.Environment, jinja_context, scripts: List[str] | ||
| ): | ||
| queued_queries = [] |
There was a problem hiding this comment.
what's a query in this context? The same doesn't sound very meaningful here. Isn't it just script content? I know the name was there before, but it's a good opportunity to clarify.
| self._execute_query(f"use database {self._conn.database}") | ||
| sql_script = snowflake_sql_jinja_render(content=sql_script) | ||
| self._execute_queries(sql_script) | ||
| if database_name: |
There was a problem hiding this comment.
nit: is not None is the typical way to check this, since we shouldn't make a habit of interpreting what it means for a string to be truthy.
Pre-review checklist
Changes description
Post_deploy script should run relative to project root, Fixes #1325