Skip to content

Commit

Permalink
Correctly identify local paths in libraries section (#702)
Browse files Browse the repository at this point in the history
## Changes
Fixes #699

## Tests
Added unit test
  • Loading branch information
andrewnester authored Aug 29, 2023
1 parent 5477afe commit 842cd8b
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 6 deletions.
36 changes: 30 additions & 6 deletions bundle/libraries/libraries.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package libraries
import (
"context"
"fmt"
"net/url"
"path/filepath"
"strings"

Expand Down Expand Up @@ -92,13 +93,13 @@ func findArtifactsAndMarkForUpload(ctx context.Context, lib *compute.Library, b
}

if len(matches) == 0 && isLocalLibrary(lib) {
return fmt.Errorf("no library found for %s", libPath(lib))
return fmt.Errorf("file %s is referenced in libraries section but doesn't exist on the local file system", libPath(lib))
}

for _, match := range matches {
af, err := findArtifactFileByLocalPath(match, b)
if err != nil {
cmdio.LogString(ctx, fmt.Sprintf("%s. Skipping %s. In order to use the library upload it manually", err.Error(), match))
cmdio.LogString(ctx, fmt.Sprintf("%s. Skipping uploading. In order to use the define 'artifacts' section", err.Error()))
} else {
af.Libraries = append(af.Libraries, lib)
}
Expand All @@ -116,7 +117,7 @@ func findArtifactFileByLocalPath(path string, b *bundle.Bundle) (*config.Artifac
}
}

return nil, fmt.Errorf("artifact file is not found for path %s", path)
return nil, fmt.Errorf("artifact section is not defined for file at %s", path)
}

func libPath(library *compute.Library) string {
Expand All @@ -139,11 +140,34 @@ func isLocalLibrary(library *compute.Library) bool {
return false
}

return !isDbfsPath(path) && !isWorkspacePath(path)
if isExplicitFileScheme(path) {
return true
}

if isRemoteStorageScheme(path) {
return false
}

return !isWorkspacePath(path)
}

func isExplicitFileScheme(path string) bool {
return strings.HasPrefix(path, "file://")
}

func isDbfsPath(path string) bool {
return strings.HasPrefix(path, "dbfs:/")
func isRemoteStorageScheme(path string) bool {
url, err := url.Parse(path)
if err != nil {
return false
}

if url.Scheme == "" {
return false
}

// If the path starts with scheme:// format, it's a correct remote storage scheme
return strings.HasPrefix(path, url.Scheme+"://")

}

func isWorkspacePath(path string) bool {
Expand Down
30 changes: 30 additions & 0 deletions bundle/libraries/libraries_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package libraries

import (
"fmt"
"testing"

"github.com/databricks/databricks-sdk-go/service/compute"
"github.com/stretchr/testify/require"
)

var testCases map[string]bool = map[string]bool{
"./some/local/path": true,
"/some/full/path": true,
"/Workspace/path/to/package": false,
"/Users/path/to/package": false,
"file://path/to/package": true,
"C:\\path\\to\\package": true,
"dbfs://path/to/package": false,
"s3://path/to/package": false,
"abfss://path/to/package": false,
}

func TestIsLocalLbrary(t *testing.T) {
for p, result := range testCases {
lib := compute.Library{
Whl: p,
}
require.Equal(t, result, isLocalLibrary(&lib), fmt.Sprintf("isLocalLibrary must return %t for path %s ", result, p))
}
}

0 comments on commit 842cd8b

Please sign in to comment.