Compare commits

...

3 Commits

Author SHA1 Message Date
shreyas-goenka 739ad57827
Merge f6232f5aae into cc112961ce 2024-10-16 19:14:42 +05:30
shreyas-goenka cc112961ce
Fix `TestAccFsMkdirWhenFileExistsAtPath` in isolated Azure environments (#1833)
## Changes
This test passes on normal `azure-prod` but started to fail on
`azure-prod-is`, which is the isolated version of azure-prod. This PR
patches the test to include the error returned from the cloud setup in
`azure-prod-is`.

## Tests
The test passes now on `azure-prod-is`.
2024-10-16 12:50:17 +00:00
Shreyas Goenka f6232f5aae
[WIP] Add support for non python ipynb notebooks 2024-10-11 16:40:15 +02:00
5 changed files with 100 additions and 37 deletions

View File

@ -360,7 +360,7 @@ func TestAccFilerReadDir(t *testing.T) {
} }
} }
var jupyterNotebookContent1 = ` var pythonJupyterNotebookContent1 = `
{ {
"cells": [ "cells": [
{ {
@ -384,7 +384,10 @@ var jupyterNotebookContent1 = `
} }
` `
var jupyterNotebookContent2 = ` // TODO: Does detect notebook work with notebooks exported from databricks?
// They typically do not have the top level "language" key set.
var pythonJupyterNotebookContent2 = `
{ {
"cells": [ "cells": [
{ {
@ -408,6 +411,10 @@ var jupyterNotebookContent2 = `
} }
` `
// TODO: Continue adding tests for other types of notebooks than python here.
// Exporting from a workspace makes this work easier.
func TestAccFilerWorkspaceNotebookConflict(t *testing.T) { func TestAccFilerWorkspaceNotebookConflict(t *testing.T) {
t.Parallel() t.Parallel()
@ -424,7 +431,7 @@ func TestAccFilerWorkspaceNotebookConflict(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
err = f.Write(ctx, "scalaNb.scala", strings.NewReader("// Databricks notebook source\n println(\"first upload\"))")) err = f.Write(ctx, "scalaNb.scala", strings.NewReader("// Databricks notebook source\n println(\"first upload\"))"))
require.NoError(t, err) require.NoError(t, err)
err = f.Write(ctx, "jupyterNb.ipynb", strings.NewReader(jupyterNotebookContent1)) err = f.Write(ctx, "pythonJupyterNb.ipynb", strings.NewReader(pythonJupyterNotebookContent1))
require.NoError(t, err) require.NoError(t, err)
// Assert contents after initial upload // Assert contents after initial upload
@ -432,7 +439,7 @@ func TestAccFilerWorkspaceNotebookConflict(t *testing.T) {
filerTest{t, f}.assertContents(ctx, "rNb", "# Databricks notebook source\nprint('first upload'))") filerTest{t, f}.assertContents(ctx, "rNb", "# Databricks notebook source\nprint('first upload'))")
filerTest{t, f}.assertContents(ctx, "sqlNb", "-- Databricks notebook source\n SELECT \"first upload\"") filerTest{t, f}.assertContents(ctx, "sqlNb", "-- Databricks notebook source\n SELECT \"first upload\"")
filerTest{t, f}.assertContents(ctx, "scalaNb", "// Databricks notebook source\n println(\"first upload\"))") filerTest{t, f}.assertContents(ctx, "scalaNb", "// Databricks notebook source\n println(\"first upload\"))")
filerTest{t, f}.assertContents(ctx, "jupyterNb", "# Databricks notebook source\nprint(\"Jupyter Notebook Version 1\")") filerTest{t, f}.assertContents(ctx, "pythonJupyterNb", "# Databricks notebook source\nprint(\"Jupyter Notebook Version 1\")")
// Assert uploading a second time fails due to overwrite mode missing // Assert uploading a second time fails due to overwrite mode missing
err = f.Write(ctx, "pyNb.py", strings.NewReader("# Databricks notebook source\nprint('second upload'))")) err = f.Write(ctx, "pyNb.py", strings.NewReader("# Databricks notebook source\nprint('second upload'))"))
@ -451,9 +458,9 @@ func TestAccFilerWorkspaceNotebookConflict(t *testing.T) {
assert.ErrorIs(t, err, fs.ErrExist) assert.ErrorIs(t, err, fs.ErrExist)
assert.Regexp(t, regexp.MustCompile(`file already exists: .*/scalaNb$`), err.Error()) assert.Regexp(t, regexp.MustCompile(`file already exists: .*/scalaNb$`), err.Error())
err = f.Write(ctx, "jupyterNb.ipynb", strings.NewReader(jupyterNotebookContent2)) err = f.Write(ctx, "pythonJupyterNb.ipynb", strings.NewReader(pythonJupyterNotebookContent2))
assert.ErrorIs(t, err, fs.ErrExist) assert.ErrorIs(t, err, fs.ErrExist)
assert.Regexp(t, regexp.MustCompile(`file already exists: .*/jupyterNb$`), err.Error()) assert.Regexp(t, regexp.MustCompile(`file already exists: .*/pythonJupyterNb$`), err.Error())
} }
func TestAccFilerWorkspaceNotebookWithOverwriteFlag(t *testing.T) { func TestAccFilerWorkspaceNotebookWithOverwriteFlag(t *testing.T) {
@ -472,7 +479,7 @@ func TestAccFilerWorkspaceNotebookWithOverwriteFlag(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
err = f.Write(ctx, "scalaNb.scala", strings.NewReader("// Databricks notebook source\n println(\"first upload\"))")) err = f.Write(ctx, "scalaNb.scala", strings.NewReader("// Databricks notebook source\n println(\"first upload\"))"))
require.NoError(t, err) require.NoError(t, err)
err = f.Write(ctx, "jupyterNb.ipynb", strings.NewReader(jupyterNotebookContent1)) err = f.Write(ctx, "jupyterNb.ipynb", strings.NewReader(pythonJupyterNotebookContent1))
require.NoError(t, err) require.NoError(t, err)
// Assert contents after initial upload // Assert contents after initial upload
@ -491,7 +498,7 @@ func TestAccFilerWorkspaceNotebookWithOverwriteFlag(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
err = f.Write(ctx, "scalaNb.scala", strings.NewReader("// Databricks notebook source\n println(\"second upload\"))"), filer.OverwriteIfExists) err = f.Write(ctx, "scalaNb.scala", strings.NewReader("// Databricks notebook source\n println(\"second upload\"))"), filer.OverwriteIfExists)
require.NoError(t, err) require.NoError(t, err)
err = f.Write(ctx, "jupyterNb.ipynb", strings.NewReader(jupyterNotebookContent2), filer.OverwriteIfExists) err = f.Write(ctx, "jupyterNb.ipynb", strings.NewReader(pythonJupyterNotebookContent2), filer.OverwriteIfExists)
require.NoError(t, err) require.NoError(t, err)
// Assert contents have been overwritten // Assert contents have been overwritten
@ -515,8 +522,8 @@ func TestAccFilerWorkspaceFilesExtensionsReadDir(t *testing.T) {
{"foo.r", "print('foo')"}, {"foo.r", "print('foo')"},
{"foo.scala", "println('foo')"}, {"foo.scala", "println('foo')"},
{"foo.sql", "SELECT 'foo'"}, {"foo.sql", "SELECT 'foo'"},
{"jupyterNb.ipynb", jupyterNotebookContent1}, {"jupyterNb.ipynb", pythonJupyterNotebookContent1},
{"jupyterNb2.ipynb", jupyterNotebookContent2}, {"jupyterNb2.ipynb", pythonJupyterNotebookContent2},
{"pyNb.py", "# Databricks notebook source\nprint('first upload'))"}, {"pyNb.py", "# Databricks notebook source\nprint('first upload'))"},
{"rNb.r", "# Databricks notebook source\nprint('first upload'))"}, {"rNb.r", "# Databricks notebook source\nprint('first upload'))"},
{"scalaNb.scala", "// Databricks notebook source\n println(\"first upload\"))"}, {"scalaNb.scala", "// Databricks notebook source\n println(\"first upload\"))"},
@ -582,7 +589,7 @@ func setupFilerWithExtensionsTest(t *testing.T) filer.Filer {
}{ }{
{"foo.py", "# Databricks notebook source\nprint('first upload'))"}, {"foo.py", "# Databricks notebook source\nprint('first upload'))"},
{"bar.py", "print('foo')"}, {"bar.py", "print('foo')"},
{"jupyter.ipynb", jupyterNotebookContent1}, {"jupyter.ipynb", pythonJupyterNotebookContent1},
{"pretender", "not a notebook"}, {"pretender", "not a notebook"},
{"dir/file.txt", "file content"}, {"dir/file.txt", "file content"},
{"scala-notebook.scala", "// Databricks notebook source\nprintln('first upload')"}, {"scala-notebook.scala", "// Databricks notebook source\nprintln('first upload')"},
@ -756,7 +763,7 @@ func TestAccWorkspaceFilesExtensions_ExportFormatIsPreserved(t *testing.T) {
assert.ErrorIs(t, err, fs.ErrNotExist) assert.ErrorIs(t, err, fs.ErrNotExist)
// Case 2: Jupyter Notebook // Case 2: Jupyter Notebook
err = wf.Write(ctx, "bar.ipynb", strings.NewReader(jupyterNotebookContent1)) err = wf.Write(ctx, "bar.ipynb", strings.NewReader(pythonJupyterNotebookContent1))
require.NoError(t, err) require.NoError(t, err)
// The Jupyter notebook should exist but not the source notebook // The Jupyter notebook should exist but not the source notebook

View File

@ -112,8 +112,8 @@ func TestAccFsMkdirWhenFileExistsAtPath(t *testing.T) {
// assert mkdir fails // assert mkdir fails
_, _, err = RequireErrorRun(t, "fs", "mkdir", path.Join(tmpDir, "hello")) _, _, err = RequireErrorRun(t, "fs", "mkdir", path.Join(tmpDir, "hello"))
// Different cloud providers return different errors. // Different cloud providers or cloud configurations return different errors.
regex := regexp.MustCompile(`(^|: )Path is a file: .*$|(^|: )Cannot create directory .* because .* is an existing file\.$|(^|: )mkdirs\(hadoopPath: .*, permission: rwxrwxrwx\): failed$`) regex := regexp.MustCompile(`(^|: )Path is a file: .*$|(^|: )Cannot create directory .* because .* is an existing file\.$|(^|: )mkdirs\(hadoopPath: .*, permission: rwxrwxrwx\): failed$|(^|: )"The specified path already exists.".*$`)
assert.Regexp(t, regex, err.Error()) assert.Regexp(t, regex, err.Error())
}) })

View File

@ -20,6 +20,7 @@ import (
"time" "time"
"github.com/databricks/cli/cmd/root" "github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/flags"
"github.com/databricks/cli/cmd" "github.com/databricks/cli/cmd"
@ -561,12 +562,10 @@ func setupLocalFiler(t *testing.T) (filer.Filer, string) {
} }
func setupWsfsFiler(t *testing.T) (filer.Filer, string) { func setupWsfsFiler(t *testing.T) (filer.Filer, string) {
t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) ctx, w := acc.WorkspaceTest(t)
ctx := context.Background() tmpdir := TemporaryWorkspaceDir(t, w.W)
w := databricks.Must(databricks.NewWorkspaceClient()) f, err := filer.NewWorkspaceFilesClient(w.W, tmpdir)
tmpdir := TemporaryWorkspaceDir(t, w)
f, err := filer.NewWorkspaceFilesClient(w, tmpdir)
require.NoError(t, err) require.NoError(t, err)
// Check if we can use this API here, skip test if we cannot. // Check if we can use this API here, skip test if we cannot.
@ -591,13 +590,10 @@ func setupWsfsExtensionsFiler(t *testing.T) (filer.Filer, string) {
} }
func setupDbfsFiler(t *testing.T) (filer.Filer, string) { func setupDbfsFiler(t *testing.T) (filer.Filer, string) {
t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) _, wt := acc.WorkspaceTest(t)
w, err := databricks.NewWorkspaceClient() tmpDir := TemporaryDbfsDir(t, wt.W)
require.NoError(t, err) f, err := filer.NewDbfsClient(wt.W, tmpDir)
tmpDir := TemporaryDbfsDir(t, w)
f, err := filer.NewDbfsClient(w, tmpDir)
require.NoError(t, err) require.NoError(t, err)
return f, path.Join("dbfs:/", tmpDir) return f, path.Join("dbfs:/", tmpDir)

View File

@ -28,6 +28,9 @@ var extensionsToLanguages = map[string]workspace.Language{
".r": workspace.LanguageR, ".r": workspace.LanguageR,
".scala": workspace.LanguageScala, ".scala": workspace.LanguageScala,
".sql": workspace.LanguageSql, ".sql": workspace.LanguageSql,
// The platform supports all languages (Python, R, Scala, and SQL) for Jupyter notebooks.
// Thus, we do not need to check the language for .ipynb files.
".ipynb": workspace.LanguagePython, ".ipynb": workspace.LanguagePython,
} }
@ -47,6 +50,10 @@ func (w *workspaceFilesExtensionsClient) stat(ctx context.Context, name string)
return info.(wsfsFileInfo), err return info.(wsfsFileInfo), err
} }
// TODO: Add end to end tests that the filer works for all .ipynb cases.
// TODO: Also fix the sync issues. OR add tests that sync works fine with non
// python notebooks. Is this needed in the first place?
// This function returns the stat for the provided notebook. The stat object itself contains the path // This function returns the stat for the provided notebook. The stat object itself contains the path
// with the extension since it is meant to be used in the context of a fs.FileInfo. // with the extension since it is meant to be used in the context of a fs.FileInfo.
func (w *workspaceFilesExtensionsClient) getNotebookStatByNameWithExt(ctx context.Context, name string) (*workspaceFileStatus, error) { func (w *workspaceFilesExtensionsClient) getNotebookStatByNameWithExt(ctx context.Context, name string) (*workspaceFileStatus, error) {
@ -75,8 +82,9 @@ func (w *workspaceFilesExtensionsClient) getNotebookStatByNameWithExt(ctx contex
return nil, nil return nil, nil
} }
// Not the correct language. Return early. // Not the correct language. Return early. Note: All languages are supported
if stat.Language != extensionsToLanguages[ext] { // for Jupyter notebooks.
if ext != ".ipynb" && stat.Language != extensionsToLanguages[ext] {
log.Debugf(ctx, "attempting to determine if %s could be a notebook. Found a notebook at %s but it is not of the correct language. Expected %s but found %s.", name, path.Join(w.root, nameWithoutExt), extensionsToLanguages[ext], stat.Language) log.Debugf(ctx, "attempting to determine if %s could be a notebook. Found a notebook at %s but it is not of the correct language. Expected %s but found %s.", name, path.Join(w.root, nameWithoutExt), extensionsToLanguages[ext], stat.Language)
return nil, nil return nil, nil
} }
@ -120,7 +128,8 @@ func (w *workspaceFilesExtensionsClient) getNotebookStatByNameWithoutExt(ctx con
ext := notebook.GetExtensionByLanguage(&stat.ObjectInfo) ext := notebook.GetExtensionByLanguage(&stat.ObjectInfo)
// If the notebook was exported as a Jupyter notebook, the extension should be .ipynb. // If the notebook was exported as a Jupyter notebook, the extension should be .ipynb.
if stat.Language == workspace.LanguagePython && stat.ReposExportFormat == workspace.ExportFormatJupyter { // TODO: Test this.
if stat.ReposExportFormat == workspace.ExportFormatJupyter {
ext = ".ipynb" ext = ".ipynb"
} }

View File

@ -44,15 +44,6 @@ func TestFilerWorkspaceFilesExtensionsErrorsOnDupName(t *testing.T) {
filePath: "/dir/foo.py", filePath: "/dir/foo.py",
expectedError: "failed to read files from the workspace file system. Duplicate paths encountered. Both NOTEBOOK at /dir/foo and FILE at /dir/foo.py resolve to the same name /foo.py. Changing the name of one of these objects will resolve this issue", expectedError: "failed to read files from the workspace file system. Duplicate paths encountered. Both NOTEBOOK at /dir/foo and FILE at /dir/foo.py resolve to the same name /foo.py. Changing the name of one of these objects will resolve this issue",
}, },
{
name: "python jupyter notebook and file",
language: workspace.LanguagePython,
notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo",
filePath: "/dir/foo.py",
// Jupyter notebooks would correspond to foo.ipynb so an error is not expected.
expectedError: "",
},
{ {
name: "scala source notebook and file", name: "scala source notebook and file",
language: workspace.LanguageScala, language: workspace.LanguageScala,
@ -82,6 +73,66 @@ func TestFilerWorkspaceFilesExtensionsErrorsOnDupName(t *testing.T) {
language: workspace.LanguagePython, language: workspace.LanguagePython,
notebookExportFormat: workspace.ExportFormatJupyter, notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo", notebookPath: "/dir/foo",
filePath: "/dir/foo.py",
// Jupyter notebooks would correspond to foo.ipynb so an error is not expected.
expectedError: "",
},
{
name: "scala jupyter notebook and file",
language: workspace.LanguageScala,
notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo",
filePath: "/dir/foo.scala",
// Jupyter notebooks would correspond to foo.ipynb so an error is not expected.
expectedError: "",
},
{
name: "sql jupyter notebook and file",
language: workspace.LanguageSql,
notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo",
filePath: "/dir/foo.sql",
// Jupyter notebooks would correspond to foo.ipynb so an error is not expected.
expectedError: "",
},
{
name: "r jupyter notebook and file",
language: workspace.LanguageR,
notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo",
filePath: "/dir/foo.sql",
// Jupyter notebooks would correspond to foo.ipynb so an error is not expected.
expectedError: "",
},
{
name: "python jupyter notebook and file",
language: workspace.LanguagePython,
notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo",
filePath: "/dir/foo.ipynb",
expectedError: "failed to read files from the workspace file system. Duplicate paths encountered. Both NOTEBOOK at /dir/foo and FILE at /dir/foo.ipynb resolve to the same name /foo.ipynb. Changing the name of one of these objects will resolve this issue",
},
{
name: "scala jupyter notebook and file",
language: workspace.LanguageScala,
notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo",
filePath: "/dir/foo.ipynb",
expectedError: "failed to read files from the workspace file system. Duplicate paths encountered. Both NOTEBOOK at /dir/foo and FILE at /dir/foo.ipynb resolve to the same name /foo.ipynb. Changing the name of one of these objects will resolve this issue",
},
{
name: "r jupyter notebook and file",
language: workspace.LanguageR,
notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo",
filePath: "/dir/foo.ipynb",
expectedError: "failed to read files from the workspace file system. Duplicate paths encountered. Both NOTEBOOK at /dir/foo and FILE at /dir/foo.ipynb resolve to the same name /foo.ipynb. Changing the name of one of these objects will resolve this issue",
},
{
name: "sql jupyter notebook and file",
language: workspace.LanguageSql,
notebookExportFormat: workspace.ExportFormatJupyter,
notebookPath: "/dir/foo",
filePath: "/dir/foo.ipynb", filePath: "/dir/foo.ipynb",
expectedError: "failed to read files from the workspace file system. Duplicate paths encountered. Both NOTEBOOK at /dir/foo and FILE at /dir/foo.ipynb resolve to the same name /foo.ipynb. Changing the name of one of these objects will resolve this issue", expectedError: "failed to read files from the workspace file system. Duplicate paths encountered. Both NOTEBOOK at /dir/foo and FILE at /dir/foo.ipynb resolve to the same name /foo.ipynb. Changing the name of one of these objects will resolve this issue",
}, },