SNOW-1621834 Cast version to identifier when creating/dropping app versions #1475
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -96,13 +96,13 @@ def test_process_has_no_existing_app_pkg(mock_get_existing, policy_param, temp_d | |||
| def test_process_no_version_from_user_no_version_in_manifest( | |||
| mock_version_info_in_manifest, | |||
| mock_build_bundle, | |||
| mock_mismatch, | |||
There was a problem hiding this comment.
not sure why this was named that
| mock_get_existing, | ||
| policy_param, | ||
| temp_dir, | ||
| ): | ||
|
|
||
| mock_mismatch.return_Value = "internal" |
There was a problem hiding this comment.
this didn't break the test 😅
sfc-gh-fcampbell
changed the title
| @@ -328,6 +327,9 @@ def process( | |||
| "Manifest.yml file does not contain a value for the version field." | |||
| ) | |||
|
|
|||
| # Make the version a valid identifier, adding quotes if necessary | |||
| version = to_identifier(version) | |||
There was a problem hiding this comment.
any way we can unify this to get_version() in manager or processor so we don't have to remember to do it every time, similar to get_app() or get_package() ?
There was a problem hiding this comment.
this also needs to handle the string passed in by the user, it's not part of the project model. I could change this to a CLI callback that casts it at that point though that wouldn't handle the version name in the manifest in case it's missing the quotes
sfc-gh-fcampbell
force-pushed
the
frank-quote-version-number
branch
from
0856ee7 to
05bff5b
Compare
sfc-gh-fcampbell
force-pushed
the
frank-quote-version-number
branch
from
05bff5b to
d9d50f2
Compare
sfc-gh-melnacouzi
approved these changes
Aug 23, 2024
sfc-gh-jsikorski
added a commit
that referenced
this pull request
Aug 25, 2024
SNOW-1636849 Auto-teardown Native App in integration tests (#1478) Changes `with project_directory()` to `with nativeapp_project_directory()`, which automatically runs `snow app teardown` before exiting the project. This allows us to remove the `try`/`finally` in most tests. For tests that were using `with pushd(test_project)`, this has been changed to `with nativeapp_teardown()`, which is what `with nativeapp_project_directory()` uses under the hood. SNOW-1621834 Cast version to identifier when creating/dropping app versions (#1475) When running `snow app version create` and `snow app version drop`, wrap the version in `to_identifier()` so users don't have to specify the quotes around version names that aren't valid identifiers. If the name is already quoted, `to_identifier()` doesn't do anything. Added tests Added tests
sfc-gh-jsikorski
added a commit
that referenced
this pull request
Aug 26, 2024
* Basic solution SNOW-1636849 Auto-teardown Native App in integration tests (#1478) Changes `with project_directory()` to `with nativeapp_project_directory()`, which automatically runs `snow app teardown` before exiting the project. This allows us to remove the `try`/`finally` in most tests. For tests that were using `with pushd(test_project)`, this has been changed to `with nativeapp_teardown()`, which is what `with nativeapp_project_directory()` uses under the hood. SNOW-1621834 Cast version to identifier when creating/dropping app versions (#1475) When running `snow app version create` and `snow app version drop`, wrap the version in `to_identifier()` so users don't have to specify the quotes around version names that aren't valid identifiers. If the name is already quoted, `to_identifier()` doesn't do anything. Added tests Added tests * Added tests Added tests * Post-review-fixes
sfc-gh-jvasquezrojas
pushed a commit
that referenced
this pull request
Aug 26, 2024
* Basic solution SNOW-1636849 Auto-teardown Native App in integration tests (#1478) Changes `with project_directory()` to `with nativeapp_project_directory()`, which automatically runs `snow app teardown` before exiting the project. This allows us to remove the `try`/`finally` in most tests. For tests that were using `with pushd(test_project)`, this has been changed to `with nativeapp_teardown()`, which is what `with nativeapp_project_directory()` uses under the hood. SNOW-1621834 Cast version to identifier when creating/dropping app versions (#1475) When running `snow app version create` and `snow app version drop`, wrap the version in `to_identifier()` so users don't have to specify the quotes around version names that aren't valid identifiers. If the name is already quoted, `to_identifier()` doesn't do anything. Added tests Added tests * Added tests Added tests * Post-review-fixes
sfc-gh-jvasquezrojas
pushed a commit
that referenced
this pull request
Aug 26, 2024
* Basic solution SNOW-1636849 Auto-teardown Native App in integration tests (#1478) Changes `with project_directory()` to `with nativeapp_project_directory()`, which automatically runs `snow app teardown` before exiting the project. This allows us to remove the `try`/`finally` in most tests. For tests that were using `with pushd(test_project)`, this has been changed to `with nativeapp_teardown()`, which is what `with nativeapp_project_directory()` uses under the hood. SNOW-1621834 Cast version to identifier when creating/dropping app versions (#1475) When running `snow app version create` and `snow app version drop`, wrap the version in `to_identifier()` so users don't have to specify the quotes around version names that aren't valid identifiers. If the name is already quoted, `to_identifier()` doesn't do anything. Added tests Added tests * Added tests Added tests * Post-review-fixes
sfc-gh-sichen
pushed a commit
that referenced
this pull request
Oct 17, 2024
…rsions (#1475) When running `snow app version create` and `snow app version drop`, wrap the version in `to_identifier()` so users don't have to specify the quotes around version names that aren't valid identifiers. If the name is already quoted, `to_identifier()` doesn't do anything.
sfc-gh-sichen
pushed a commit
that referenced
this pull request
Oct 17, 2024
* Basic solution SNOW-1636849 Auto-teardown Native App in integration tests (#1478) Changes `with project_directory()` to `with nativeapp_project_directory()`, which automatically runs `snow app teardown` before exiting the project. This allows us to remove the `try`/`finally` in most tests. For tests that were using `with pushd(test_project)`, this has been changed to `with nativeapp_teardown()`, which is what `with nativeapp_project_directory()` uses under the hood. SNOW-1621834 Cast version to identifier when creating/dropping app versions (#1475) When running `snow app version create` and `snow app version drop`, wrap the version in `to_identifier()` so users don't have to specify the quotes around version names that aren't valid identifiers. If the name is already quoted, `to_identifier()` doesn't do anything. Added tests Added tests * Added tests Added tests * Post-review-fixes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pre-review checklist
Changes description
When running
snow app version createandsnow app version drop, wrap the version into_identifier()so users don't have to specify the quotes around version names that aren't valid identifiers. If the name is already quoted,to_identifier()doesn't do anything.