-
Notifications
You must be signed in to change notification settings - Fork 54
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
SNOW-1706990 Convert test_manager.py to use v2 entities #1722
Conversation
@@ -266,7 +272,7 @@ def action_validate( | |||
post_deploy_hooks=model.meta and model.meta.post_deploy, | |||
package_scripts=[], # Package scripts are not supported in PDFv2 | |||
policy=policy, | |||
use_scratch_stage=True, | |||
use_scratch_stage=use_scratch_stage, |
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.
had to make this a param since we want False
in tests
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.
why is that? Didn't get it from the diff.
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.
in tests we don't want to call deploy()
on the package, so we need to be able to pass False
here. In the old NativeAppManager.validate()
, the use_scratch_stage
parameter defaults to False
:
def validate(self, use_scratch_stage: bool = False): |
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 doesn't change prod code since the parameter now defaults to True
in v2, the same value that was hardcoded here before
Updates tests in `test_manager.py` to use a v2 test project and call out to the v2 entities directly instead of going through the `NativeAppManager`. I've kept the diff as simple as possible and avoided introducing too many helpers or reorganizing the code, that can be done separately if necessary (the top priority is to just work towards removing `NativeAppManager` and all these static methods first).
Pre-review checklist
Changes description
Updates tests in
test_manager.py
to use a v2 test project and call out to the v2 entities directly instead of going through theNativeAppManager
. I've kept the diff as simple as possible and avoided introducing too many helpers or reorganizing the code, that can be done separately if necessary (the top priority is to just work towards removingNativeAppManager
and all these static methods first).