SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags #826

sfc-gh-davwang · 2024-02-24T01:29:26Z

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.
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 generic flags for REPLACE. IF EXISTS and IF NOT EXISTS to flags.py. Added these flags to spcs service create, spcs compute-pool create, and spcs image-repository create. Also replaced existing usages of --replace with the new flag.

Usage:

02/26/24: Added validation that at most one create mode option is set to true

sfc-gh-davwang · 2024-02-24T05:38:52Z

src/snowflake/cli/api/commands/flags.py

+        passed_kwargs.pop("param_decls", None)
+        return typer.Option(default, *param_decls, **passed_kwargs)
+
+


this chunk moves OverrideableOption to the top of the file and also implements mutually_exclusive and _callback_factory

src/snowflake/cli/plugins/spcs/image_repository/manager.py

sfc-gh-cgorrie

I would like to approve because it provides a lot of customer value, but am not doing so because NADE has no ownership over the affected files. Added some food-for-thought comments... if your time is limited I'm happy to take this PR over but first we need some feedback from the Snowflake CLI folks.

src/snowflake/cli/api/commands/flags.py

src/snowflake/cli/plugins/spcs/image_repository/manager.py

tests/api/commands/test_flags.py

sfc-gh-astus · 2024-03-01T11:33:47Z

tests/api/commands/test_flags.py

+
+
+def test_overrideable_option_invalid_callback_signature():
+    pass


Is this test needed?

tests/spcs/test_image_repository.py

tests/api/commands/test_flags.py

sfc-gh-astus · 2024-03-01T12:10:38Z

src/snowflake/cli/api/commands/flags.py

@@ -13,6 +14,106 @@
 _CLI_BEHAVIOUR = "Global configuration"


+class OverrideableOption:


This class needs more tests, has a lot of ifs which should be tested.

good point, I've added more tests to test_flags but let me know if there's any cases I haven't thought of

…gs to flags.py

…amlit deploy

…and updating unit tests.

…pdating unit tests

…age-repository create.

…be set at once

…age-repository create to throw error

…tion

This reverts commit 80467b3. mraba/app-factory: create app with factory (#860) * mraba/app-factory: create app with factory SNOW-1043081: Adding support for qualified names for image repositories. (#823) * SNOW-1043081: Adding support for qualified image repository names * SNOW-1043081: Fixing test imports * SNOW-1043081: Adding tests for getting image repository url without db or schema SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags (#826) * SNOW-1011771: Adding generic OR REPLACE, IF EXISTS, IF NOT EXISTS flags to flags.py * SNOW-1011771: Using generic ReplaceOption in snowpark deploy and streamlit deploy * SNOW-1011771: Using generic IfNotExistsOption in compute pool create and updating unit tests. * SNOW-1011771: Using generic IfNotExistsOption in service create and updating unit tests * SNOW-1011771: Using generic ReplaceOption and IfNotExistsOption in image-repository create. * SNOW-1011771: Fixup * SNOW-1011771: Update release notes * SNOW-1011771: Update test_help_messages * SNOW-1011771: precommit * SNOW-1011771: Adding validation that only one create mode option can be set at once * fixup * SNOW-1011771: Updating tests for REPLACE AND IF NOT EXISTS case on image-repository create to throw error * SNOW-1011771: Adding snapshots * SNOW-1011771: Adding a new mutually_exclusive field to OverrideableOption * formatting * SNOW-1011771: Adding tests for OverrideableOption * SNOW-1011771: Fixing test failures due to improperly quoted string Add snow --help to test_help_messages (#821) * Add snow --help to test_help_messages * update snapshot Avoid plain print, make sure silent is eager flag (#871) [NADE] Update CODEOWNERS to use NADE team id. (#873) update to using nade team in codeowners New workflow to stop running workflows after new commit (#862) * new workflow * new workflow * new workflow * new workflow * typo fix * typo fix * import fix * import fix * import fix * import fix * import fix * import fix * import fix * new approach * new approach * new approach * new approach * new approach * New approach * added to test * Added to more workflows * Dummy commit Schemas adjusting native apps to streamlit fixing streamlit fixies after unit tests fixies after unit tests fixing for snowflake fixing for snowflake Fixes after review Fixes after review Fixes after review

* Revert "Bump tomlkit from 0.12.3 to 0.12.4 (#848)" (#863) This reverts commit 80467b3. mraba/app-factory: create app with factory (#860) * mraba/app-factory: create app with factory SNOW-1043081: Adding support for qualified names for image repositories. (#823) * SNOW-1043081: Adding support for qualified image repository names * SNOW-1043081: Fixing test imports * SNOW-1043081: Adding tests for getting image repository url without db or schema SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags (#826) * SNOW-1011771: Adding generic OR REPLACE, IF EXISTS, IF NOT EXISTS flags to flags.py * SNOW-1011771: Using generic ReplaceOption in snowpark deploy and streamlit deploy * SNOW-1011771: Using generic IfNotExistsOption in compute pool create and updating unit tests. * SNOW-1011771: Using generic IfNotExistsOption in service create and updating unit tests * SNOW-1011771: Using generic ReplaceOption and IfNotExistsOption in image-repository create. * SNOW-1011771: Fixup * SNOW-1011771: Update release notes * SNOW-1011771: Update test_help_messages * SNOW-1011771: precommit * SNOW-1011771: Adding validation that only one create mode option can be set at once * fixup * SNOW-1011771: Updating tests for REPLACE AND IF NOT EXISTS case on image-repository create to throw error * SNOW-1011771: Adding snapshots * SNOW-1011771: Adding a new mutually_exclusive field to OverrideableOption * formatting * SNOW-1011771: Adding tests for OverrideableOption * SNOW-1011771: Fixing test failures due to improperly quoted string Add snow --help to test_help_messages (#821) * Add snow --help to test_help_messages * update snapshot Avoid plain print, make sure silent is eager flag (#871) [NADE] Update CODEOWNERS to use NADE team id. (#873) update to using nade team in codeowners New workflow to stop running workflows after new commit (#862) * new workflow * new workflow * new workflow * new workflow * typo fix * typo fix * import fix * import fix * import fix * import fix * import fix * import fix * import fix * new approach * new approach * new approach * new approach * new approach * New approach * added to test * Added to more workflows * Dummy commit Schemas adjusting native apps to streamlit fixing streamlit fixies after unit tests fixies after unit tests fixing for snowflake fixing for snowflake Fixes after review Fixes after review Fixes after review * Fixes after review * Implemented error class * Fixes * Fixes * Fixes * Fixes * typo fix * Added unit test * Added unit test * Fixes after review * Fixes after review * Fixes * Fixes * Fixes --------- Co-authored-by: Adam Stus <[email protected]>

…826) * SNOW-1011771: Adding generic OR REPLACE, IF EXISTS, IF NOT EXISTS flags to flags.py * SNOW-1011771: Using generic ReplaceOption in snowpark deploy and streamlit deploy * SNOW-1011771: Using generic IfNotExistsOption in compute pool create and updating unit tests. * SNOW-1011771: Using generic IfNotExistsOption in service create and updating unit tests * SNOW-1011771: Using generic ReplaceOption and IfNotExistsOption in image-repository create. * SNOW-1011771: Fixup * SNOW-1011771: Update release notes * SNOW-1011771: Update test_help_messages * SNOW-1011771: precommit * SNOW-1011771: Adding validation that only one create mode option can be set at once * fixup * SNOW-1011771: Updating tests for REPLACE AND IF NOT EXISTS case on image-repository create to throw error * SNOW-1011771: Adding snapshots * SNOW-1011771: Adding a new mutually_exclusive field to OverrideableOption * formatting * SNOW-1011771: Adding tests for OverrideableOption * SNOW-1011771: Fixing test failures due to improperly quoted string

* Revert "Bump tomlkit from 0.12.3 to 0.12.4 (#848)" (#863) This reverts commit a3ed7cc. mraba/app-factory: create app with factory (#860) * mraba/app-factory: create app with factory SNOW-1043081: Adding support for qualified names for image repositories. (#823) * SNOW-1043081: Adding support for qualified image repository names * SNOW-1043081: Fixing test imports * SNOW-1043081: Adding tests for getting image repository url without db or schema SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags (#826) * SNOW-1011771: Adding generic OR REPLACE, IF EXISTS, IF NOT EXISTS flags to flags.py * SNOW-1011771: Using generic ReplaceOption in snowpark deploy and streamlit deploy * SNOW-1011771: Using generic IfNotExistsOption in compute pool create and updating unit tests. * SNOW-1011771: Using generic IfNotExistsOption in service create and updating unit tests * SNOW-1011771: Using generic ReplaceOption and IfNotExistsOption in image-repository create. * SNOW-1011771: Fixup * SNOW-1011771: Update release notes * SNOW-1011771: Update test_help_messages * SNOW-1011771: precommit * SNOW-1011771: Adding validation that only one create mode option can be set at once * fixup * SNOW-1011771: Updating tests for REPLACE AND IF NOT EXISTS case on image-repository create to throw error * SNOW-1011771: Adding snapshots * SNOW-1011771: Adding a new mutually_exclusive field to OverrideableOption * formatting * SNOW-1011771: Adding tests for OverrideableOption * SNOW-1011771: Fixing test failures due to improperly quoted string Add snow --help to test_help_messages (#821) * Add snow --help to test_help_messages * update snapshot Avoid plain print, make sure silent is eager flag (#871) [NADE] Update CODEOWNERS to use NADE team id. (#873) update to using nade team in codeowners New workflow to stop running workflows after new commit (#862) * new workflow * new workflow * new workflow * new workflow * typo fix * typo fix * import fix * import fix * import fix * import fix * import fix * import fix * import fix * new approach * new approach * new approach * new approach * new approach * New approach * added to test * Added to more workflows * Dummy commit Schemas adjusting native apps to streamlit fixing streamlit fixies after unit tests fixies after unit tests fixing for snowflake fixing for snowflake Fixes after review Fixes after review Fixes after review * Fixes after review * Implemented error class * Fixes * Fixes * Fixes * Fixes * typo fix * Added unit test * Added unit test * Fixes after review * Fixes after review * Fixes * Fixes * Fixes --------- Co-authored-by: Adam Stus <[email protected]>

sfc-gh-davwang commented Feb 24, 2024

View reviewed changes

sfc-gh-davwang marked this pull request as ready for review February 24, 2024 05:57

sfc-gh-davwang requested review from sfc-gh-turbaszek, sfc-gh-pjob, sfc-gh-jsikorski, sfc-gh-astus, sfc-gh-mraba and sfc-gh-pczajka as code owners February 24, 2024 05:57

sfc-gh-davwang force-pushed the SNOW-1011771-create-exists-replace branch from 3f4282c to 09a9606 Compare February 26, 2024 18:54

sfc-gh-cgorrie reviewed Feb 26, 2024

View reviewed changes

src/snowflake/cli/plugins/spcs/image_repository/manager.py Outdated Show resolved Hide resolved

sfc-gh-davwang requested a review from sfc-gh-yiwang February 26, 2024 21:18

sfc-gh-cgorrie reviewed Feb 28, 2024

View reviewed changes

src/snowflake/cli/api/commands/flags.py Outdated Show resolved Hide resolved

src/snowflake/cli/plugins/spcs/image_repository/manager.py Outdated Show resolved Hide resolved

sfc-gh-davwang force-pushed the SNOW-1011771-create-exists-replace branch from 7dadbf8 to c52e33a Compare February 28, 2024 20:13

sfc-gh-davwang requested a review from sfc-gh-cgorrie February 29, 2024 04:21

sfc-gh-davwang changed the title ~~SNOW-1011771: Added generic CREATE, IF EXISTS, IF NOT EXISTS flags~~ SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags Mar 1, 2024

sfc-gh-astus reviewed Mar 1, 2024

View reviewed changes

sfc-gh-davwang force-pushed the SNOW-1011771-create-exists-replace branch 3 times, most recently from ec9ff48 to e8f7f98 Compare March 4, 2024 08:33

sfc-gh-davwang requested a review from sfc-gh-astus March 4, 2024 09:06

sfc-gh-davwang force-pushed the SNOW-1011771-create-exists-replace branch from 4b51d67 to 9ebed5e Compare March 4, 2024 18:28

sfc-gh-davwang added 9 commits March 5, 2024 01:34

SNOW-1011771: Adding generic OR REPLACE, IF EXISTS, IF NOT EXISTS fla…

2843a4d

…gs to flags.py

SNOW-1011771: Using generic ReplaceOption in snowpark deploy and stre…

e208afc

…amlit deploy

SNOW-1011771: Using generic IfNotExistsOption in compute pool create …

a7e6908

…and updating unit tests.

SNOW-1011771: Using generic IfNotExistsOption in service create and u…

a6e57dc

…pdating unit tests

SNOW-1011771: Using generic ReplaceOption and IfNotExistsOption in im…

6a345a3

…age-repository create.

SNOW-1011771: Fixup

8f28ab1

SNOW-1011771: Update release notes

73aed81

SNOW-1011771: Update test_help_messages

bf2267c

SNOW-1011771: precommit

c32ee22

sfc-gh-davwang added 7 commits March 5, 2024 01:37

SNOW-1011771: Adding validation that only one create mode option can …

a526ebe

…be set at once

fixup

b2871b2

SNOW-1011771: Updating tests for REPLACE AND IF NOT EXISTS case on im…

0fa6528

…age-repository create to throw error

SNOW-1011771: Adding snapshots

224c454

SNOW-1011771: Adding a new mutually_exclusive field to OverrideableOp…

f017d7a

…tion

formatting

778759c

SNOW-1011771: Adding tests for OverrideableOption

1d532a8

sfc-gh-davwang force-pushed the SNOW-1011771-create-exists-replace branch from 9ebed5e to 50ee873 Compare March 5, 2024 09:37

SNOW-1011771: Fixing test failures due to improperly quoted string

d3b39bd

sfc-gh-davwang force-pushed the SNOW-1011771-create-exists-replace branch from 50ee873 to d3b39bd Compare March 5, 2024 09:40

sfc-gh-astus approved these changes Mar 5, 2024

View reviewed changes

sfc-gh-davwang merged commit 1e939d6 into main Mar 5, 2024
18 checks passed

sfc-gh-davwang deleted the SNOW-1011771-create-exists-replace branch March 5, 2024 10:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags #826

SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags #826

sfc-gh-davwang commented Feb 24, 2024 •

edited

Loading

sfc-gh-davwang Feb 24, 2024 •

edited

Loading

sfc-gh-cgorrie left a comment

sfc-gh-astus Mar 1, 2024

sfc-gh-astus Mar 1, 2024

sfc-gh-davwang Mar 4, 2024

		passed_kwargs.pop("param_decls", None)
		return typer.Option(default, param_decls, *passed_kwargs)



		def test_overrideable_option_invalid_callback_signature():
		pass

		@@ -13,6 +14,106 @@
		_CLI_BEHAVIOUR = "Global configuration"


		class OverrideableOption:

SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags #826

SNOW-1011771: Added generic REPLACE, IF EXISTS, IF NOT EXISTS flags #826

Conversation

sfc-gh-davwang commented Feb 24, 2024 • edited Loading

Pre-review checklist

Changes description

sfc-gh-davwang Feb 24, 2024 • edited Loading

Choose a reason for hiding this comment

sfc-gh-cgorrie left a comment

Choose a reason for hiding this comment

sfc-gh-astus Mar 1, 2024

Choose a reason for hiding this comment

sfc-gh-astus Mar 1, 2024

Choose a reason for hiding this comment

sfc-gh-davwang Mar 4, 2024

Choose a reason for hiding this comment

sfc-gh-davwang commented Feb 24, 2024 •

edited

Loading

sfc-gh-davwang Feb 24, 2024 •

edited

Loading