-
Notifications
You must be signed in to change notification settings - Fork 69
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
Library to validate and normalize cloud specific tags #819
Merged
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
andrewnester
approved these changes
Sep 29, 2023
lennartkats-db
approved these changes
Sep 29, 2023
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 2, 2023
## Changes The jobs backend propagates job tags to the underlying cloud provider's resources. As such, they need to match the constraints a cloud provider places on tag values. The display name can contain anything. With this change, we modify the tag value to equal the short name as used in the name prefix. Additionally, we leverage tag normalization as introduced in #819 to make sure characters that aren't accepted are removed before using the value as a tag value. This is a new stab at #810 and should completely eliminate this class of problems. ## Tests Tests pass.
Merged
andrewnester
added a commit
that referenced
this pull request
Oct 4, 2023
CLI: * Refactor change computation for sync ([#785](#785)). Bundles: * Allow digits in the generated short name ([#820](#820)). * Emit an error when incompatible all purpose cluster used with Python wheel tasks ([#823](#823)). * Use normalized short name for tag value in development mode ([#821](#821)). * Added `python.DetectInterpreters` and other utils ([#805](#805)). * Mark artifacts properties as optional ([#834](#834)). * Added support for glob patterns in pipeline libraries section ([#833](#833)). Internal: * Run tests to verify backend tag validation behavior ([#814](#814)). * Library to validate and normalize cloud specific tags ([#819](#819)). * Added test to submit and run various Python tasks on multiple DBR versions ([#806](#806)). * Create a release PR in setup-cli repo on tag push ([#827](#827)). API Changes: * Changed `databricks account metastore-assignments list` command to return . * Changed `databricks jobs cancel-all-runs` command with new required argument order. * Added `databricks account o-auth-published-apps` command group. * Changed `databricks serving-endpoints query` command . New request type is . * Added `databricks serving-endpoints patch` command. * Added `databricks credentials-manager` command group. * Added `databricks settings` command group. * Changed `databricks clean-rooms list` command to require request of . * Changed `databricks statement-execution execute-statement` command with new required argument order. OpenAPI commit bcbf6e851e3d82fd910940910dd31c10c059746c (2023-10-02) Dependency updates: * Bump github.com/google/uuid from 1.3.0 to 1.3.1 ([#825](#825)). * Updated Go SDK to 0.22.0 ([#831](#831)).
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 4, 2023
CLI: * Refactor change computation for sync ([#785](#785)). Bundles: * Allow digits in the generated short name ([#820](#820)). * Emit an error when incompatible all purpose cluster used with Python wheel tasks ([#823](#823)). * Use normalized short name for tag value in development mode ([#821](#821)). * Added `python.DetectInterpreters` and other utils ([#805](#805)). * Mark artifacts properties as optional ([#834](#834)). * Added support for glob patterns in pipeline libraries section ([#833](#833)). Internal: * Run tests to verify backend tag validation behavior ([#814](#814)). * Library to validate and normalize cloud specific tags ([#819](#819)). * Added test to submit and run various Python tasks on multiple DBR versions ([#806](#806)). * Create a release PR in setup-cli repo on tag push ([#827](#827)). API Changes: * Changed `databricks account metastore-assignments list` command to return . * Changed `databricks jobs cancel-all-runs` command with new required argument order. * Added `databricks account o-auth-published-apps` command group. * Changed `databricks serving-endpoints query` command . New request type is . * Added `databricks serving-endpoints patch` command. * Added `databricks credentials-manager` command group. * Added `databricks settings` command group. * Changed `databricks clean-rooms list` command to require request of . * Changed `databricks statement-execution execute-statement` command with new required argument order. OpenAPI commit bcbf6e851e3d82fd910940910dd31c10c059746c (2023-10-02) Dependency updates: * Bump github.com/google/uuid from 1.3.0 to 1.3.1 ([#825](#825)). * Updated Go SDK to 0.22.0 ([#831](#831)).
hectorcast-db
pushed a commit
that referenced
this pull request
Oct 13, 2023
## Changes Prompted by the proposed fix for a tagging-related problem in #810, I investigated how tag validation works. This turned out to be quite a bit more complex than anticipated. Tags at the job level (or cluster level) are passed through to the underlying compute infrastructure and as such are tested against cloud-specific validation rules. GCP appears to be the most restrictive. It would be disappointing to always restrict to `\w+`, so this package implements validation and normalization rules for each cloud. It can pick the right cloud to use using a Go SDK configuration. ## Tests Exhaustive unit tests. The regular expressions were pulled by #814.
hectorcast-db
pushed a commit
that referenced
this pull request
Oct 13, 2023
## Changes The jobs backend propagates job tags to the underlying cloud provider's resources. As such, they need to match the constraints a cloud provider places on tag values. The display name can contain anything. With this change, we modify the tag value to equal the short name as used in the name prefix. Additionally, we leverage tag normalization as introduced in #819 to make sure characters that aren't accepted are removed before using the value as a tag value. This is a new stab at #810 and should completely eliminate this class of problems. ## Tests Tests pass.
hectorcast-db
pushed a commit
that referenced
this pull request
Oct 13, 2023
CLI: * Refactor change computation for sync ([#785](#785)). Bundles: * Allow digits in the generated short name ([#820](#820)). * Emit an error when incompatible all purpose cluster used with Python wheel tasks ([#823](#823)). * Use normalized short name for tag value in development mode ([#821](#821)). * Added `python.DetectInterpreters` and other utils ([#805](#805)). * Mark artifacts properties as optional ([#834](#834)). * Added support for glob patterns in pipeline libraries section ([#833](#833)). Internal: * Run tests to verify backend tag validation behavior ([#814](#814)). * Library to validate and normalize cloud specific tags ([#819](#819)). * Added test to submit and run various Python tasks on multiple DBR versions ([#806](#806)). * Create a release PR in setup-cli repo on tag push ([#827](#827)). API Changes: * Changed `databricks account metastore-assignments list` command to return . * Changed `databricks jobs cancel-all-runs` command with new required argument order. * Added `databricks account o-auth-published-apps` command group. * Changed `databricks serving-endpoints query` command . New request type is . * Added `databricks serving-endpoints patch` command. * Added `databricks credentials-manager` command group. * Added `databricks settings` command group. * Changed `databricks clean-rooms list` command to require request of . * Changed `databricks statement-execution execute-statement` command with new required argument order. OpenAPI commit bcbf6e851e3d82fd910940910dd31c10c059746c (2023-10-02) Dependency updates: * Bump github.com/google/uuid from 1.3.0 to 1.3.1 ([#825](#825)). * Updated Go SDK to 0.22.0 ([#831](#831)).
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.
Changes
Prompted by the proposed fix for a tagging-related problem in #810, I investigated how tag validation works. This turned out to be quite a bit more complex than anticipated. Tags at the job level (or cluster level) are passed through to the underlying compute infrastructure and as such are tested against cloud-specific validation rules. GCP appears to be the most restrictive. It would be disappointing to always restrict to
\w+
, so this package implements validation and normalization rules for each cloud. It can pick the right cloud to use using a Go SDK configuration.Tests
Exhaustive unit tests. The regular expressions were pulled by #814.