-
Notifications
You must be signed in to change notification settings - Fork 58
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
V2.8.0 cherry picks #1467
V2.8.0 cherry picks #1467
Conversation
d909832
to
3519b10
Compare
pdfv1["native_app"]["package"][ | ||
"post_deploy" | ||
] = app_package_definition.meta.post_deploy | ||
if app_package_definition.meta: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line and below is not needed (role and warehouse)
new_callable=mock.PropertyMock, | ||
) | ||
@mock.patch(NATIVEAPP_MANAGER_EXECUTE) | ||
def test_stream_events(mock_execute, mock_account_event_table, temp_dir, mock_cursor): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this stream test is unrelated to my changes and can be removed.
"role": "pkg_role", | ||
"warehouse": "pkg_wh", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these 2 lines can be removed.
* Added support for `native_app.package.post_deploy` scripts in project definition file. | ||
* These scripts will execute whenever a Native App Package is created or updated. | ||
* Currently only supports SQL scripts: `post_deploy: [{sql_script: script.sql}]` | ||
* Added support for project definition file defaults in templates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line can be removed - it's duplicate.
@@ -155,30 +150,6 @@ def _execute_sql_script( | |||
except ProgrammingError as err: | |||
generic_sql_error_handler(err) | |||
|
|||
def _execute_post_deploy_hooks(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the function above (_execute_sql_script) can also be removed.
@pytest.mark.parametrize( | ||
"definition", | ||
[ | ||
{ | ||
"definition_version": "1.1", | ||
"native_app": { | ||
"name": "myapp", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests from here below (All the _suffixing tests) can be removed - they're failing now right?
@@ -0,0 +1,5 @@ | |||
-- package script (1/2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file doesn't seem to be used.
@@ -0,0 +1,12 @@ | |||
-- package script (2/2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file doesn't seem to be used.
@@ -420,94 +420,6 @@ def test_nativeapp_init_from_repo_with_single_template( | |||
single_template_repo.close() | |||
|
|||
|
|||
# Tests that application post-deploy scripts are executed by creating a post_deploy_log table and having each post-deploy script add a record to it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are a couple of imports that can be removed form this file as well.
Fixed in #1469 |
Pre-review checklist
Changes description
Branch collecting chosen changes for v2.8.0 release