Refactor jobs path translation #1782
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
kanterov
force-pushed
the
kanterov/refactor-jobs-paths
branch
2 times, most recently
from
7a24c99
to
c983bb2
Compare
kanterov
force-pushed
the
kanterov/refactor-jobs-paths
branch
from
c983bb2
to
18bec2b
Compare
andrewnester
approved these changes
Sep 24, 2024
|
Thanks @andrewnester, can you please add to the merge queue? |
pietern
reviewed
Sep 25, 2024
| PathKindWithPrefix, | ||
| func(s string) bool { | ||
| return !libraries.IsLibraryLocal(s) | ||
| }, |
There was a problem hiding this comment.
IMO this piece belongs in the original code. Then this routine (and package) is only concerned with visiting everything and what to do with these paths is something for the caller to decide. These entries can use a different Kind to differentiate what they contain.
There was a problem hiding this comment.
The rationale here is that if property specifies a PyPi library, it isn't really a path, so it doesn't need to be handled. I don't see a use-case when we want to visit PyPi libraries, but if we need that, it should be very fixable by adding a separate kind.
There was a problem hiding this comment.
Yeah, that makes sense. But then the code should be updated to no longer talk about rewriting but rather about just visiting or not visiting. It's the caller who decides what to do upon visiting.
andrewnester
added a commit
that referenced
this pull request
Oct 1, 2024
Bundles: * Added support for creating all-purpose clusters ([#1698](#1698)). * Reduce time until the prompt is shown for bundle run ([#1727](#1727)). * Use Unity Catalog for pipelines in the default-python template ([#1766](#1766)). * Add verbose flag to the "bundle deploy" command ([#1774](#1774)). * Fixed full variable override detection ([#1787](#1787)). * Add sub-extension to resource files in built-in templates ([#1777](#1777)). * Fix panic in `apply_presets.go` ([#1796](#1796)). Internal: * Assert tokens are redacted in origin URL when username is not specified ([#1785](#1785)). * Refactor jobs path translation ([#1782](#1782)). * Add JobTaskClusterSpec validate mutator ([#1784](#1784)). * Pin Go toolchain to 1.22.7 ([#1790](#1790)). * Modify SetLocation test utility to take full locations as argument ([#1788](#1788)). * Simplified isFullVariableOverrideDef implementation ([#1791](#1791)). * Sort tasks by `task_key` before generating the Terraform configuration ([#1776](#1776)). * Trim trailing whitespace ([#1794](#1794)). * Move trampoline code into trampoline package ([#1793](#1793)). * Rename `RootPath` -> `BundleRootPath` ([#1792](#1792)). API Changes: * Changed `databricks apps delete` command to return . * Changed `databricks apps deploy` command with new required argument order. * Changed `databricks apps start` command to return . * Changed `databricks apps stop` command to return . * Added `databricks temporary-table-credentials` command group. * Added `databricks serving-endpoints put-ai-gateway` command. * Added `databricks disable-legacy-access` command group. * Added `databricks account disable-legacy-features` command group. OpenAPI commit 6f6b1371e640f2dfeba72d365ac566368656f6b6 (2024-09-19) Dependency updates: * Upgrade to Go SDK 0.47.0 ([#1799](#1799)). * Upgrade to TF provider 1.52 ([#1781](#1781)). * Bump golang.org/x/mod from 0.20.0 to 0.21.0 ([#1758](#1758)). * Bump github.com/hashicorp/hc-install from 0.7.0 to 0.9.0 ([#1772](#1772)).
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 1, 2024
Bundles: * Added support for creating all-purpose clusters ([#1698](#1698)). * Reduce time until the prompt is shown for bundle run ([#1727](#1727)). * Use Unity Catalog for pipelines in the default-python template ([#1766](#1766)). * Add verbose flag to the "bundle deploy" command ([#1774](#1774)). * Fixed full variable override detection ([#1787](#1787)). * Add sub-extension to resource files in built-in templates ([#1777](#1777)). * Fix panic in `apply_presets.go` ([#1796](#1796)). Internal: * Assert tokens are redacted in origin URL when username is not specified ([#1785](#1785)). * Refactor jobs path translation ([#1782](#1782)). * Add JobTaskClusterSpec validate mutator ([#1784](#1784)). * Pin Go toolchain to 1.22.7 ([#1790](#1790)). * Modify SetLocation test utility to take full locations as argument ([#1788](#1788)). * Simplified isFullVariableOverrideDef implementation ([#1791](#1791)). * Sort tasks by `task_key` before generating the Terraform configuration ([#1776](#1776)). * Trim trailing whitespace ([#1794](#1794)). * Move trampoline code into trampoline package ([#1793](#1793)). * Rename `RootPath` -> `BundleRootPath` ([#1792](#1792)). API Changes: * Changed `databricks apps delete` command to return . * Changed `databricks apps deploy` command with new required argument order. * Changed `databricks apps start` command to return . * Changed `databricks apps stop` command to return . * Added `databricks temporary-table-credentials` command group. * Added `databricks serving-endpoints put-ai-gateway` command. * Added `databricks disable-legacy-access` command group. * Added `databricks account disable-legacy-features` command group. OpenAPI commit 6f6b1371e640f2dfeba72d365ac566368656f6b6 (2024-09-19) Dependency updates: * Upgrade to Go SDK 0.47.0 ([#1799](#1799)). * Upgrade to TF provider 1.52 ([#1781](#1781)). * Bump golang.org/x/mod from 0.20.0 to 0.21.0 ([#1758](#1758)). * Bump github.com/hashicorp/hc-install from 0.7.0 to 0.9.0 ([#1772](#1772)).
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
Extract package for other modules to transform different kinds of paths in job resources.
Tests
Unit tests