Skip to content

Added Upload and Download methods where applicable#423

Merged
nfx merged 10 commits intomainfrom
upload-download
Jun 13, 2023
Merged

Added Upload and Download methods where applicable#423
nfx merged 10 commits intomainfrom
upload-download

Conversation

@nfx
Copy link
Copy Markdown
Contributor

@nfx nfx commented Jun 9, 2023

Changes

  • adds Upload and Download methods for Workspace API
  • adds Upload and Download methods for Files API
  • removed generating direct_download parameter in downstream SDKs, as it makes return type non-deterministic

Tests

Run new integrations tests

@nfx nfx requested a review from pietern June 9, 2023 14:40
visitors = append([]func(*http.Request) error{
c.Config.Authenticate,
c.addHostToRequestUrl,
c.addApplicationJsonContentType,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this doesn't change behavior, but strictly speaking we only need to set this header on requests with a body. We can set the Accept header on all of them as it pertains to the response.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we set the multipart header or octet-stream header in request visitors. multipart requests do require a Content-Type because of the multipart boundary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not talking about why this change is done, I'm saying this header should not be set for GETs, or other requests without a request body.

}

func TestAccWorkspaceUploadNotebookWithFileExtensionNoTranspile(t *testing.T) {
// TODO: remove NoTranspile suffix once other languages get Upload/Donwload features
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where?..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Donwload

// on an analysis of the `item`’s extension and the header content provided in the request.
//
// Returns [bytes.Buffer] of the path contents.
func (a *WorkspaceAPI) Download(ctx context.Context, path string, opts ...DownloadOption) (*bytes.Buffer, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this return an io.ReadCloser as well for symmetry with the Files API.

@nfx nfx requested a review from pietern June 12, 2023 20:04
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 13, 2023

Codecov Report

Patch coverage: 5.06% and project coverage change: -0.15 ⚠️

Comparison is base (6f34230) 19.03% compared to head (da42b50) 18.89%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #423      +/-   ##
==========================================
- Coverage   19.03%   18.89%   -0.15%     
==========================================
  Files          82       82              
  Lines        8911     8988      +77     
==========================================
+ Hits         1696     1698       +2     
- Misses       7068     7141      +73     
- Partials      147      149       +2     
Impacted Files Coverage Δ
openapi/code/package.go 58.57% <0.00%> (-0.57%) ⬇️
service/workspace/model.go 0.00% <ø> (ø)
service/workspace/utilities.go 0.00% <0.00%> (ø)
client/client.go 81.65% <50.00%> (-0.98%) ⬇️
openapi/roll/tool.go 82.74% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

// on an analysis of the `item`’s extension and the file content header provided in the request.
//
// Returns [bytes.Buffer] of the path contents.
func (a *WorkspaceAPI) Download(ctx context.Context, path string, opts ...DownloadOption) (io.Reader, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this a io.ReadCloser like the Files API, even if it's a nop closer. It's needed if the downstream users ever aborts reading a stream.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these files are capped to 10Mb. why do we need a stream? :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interface parity -- then you can spot a Download method and know you get a io.ReadCloser and have to close it. The nuance difference is additional overhead (when do I close when don't I close) which is hard to spot in code review. If the contract is that Download always needs a close, it's simple.


// ReadFile is identical to [os.ReadFile] but for workspace files.
func (a *WorkspaceAPI) ReadFile(ctx context.Context, name string) ([]byte, error) {
b, err := a.Download(ctx, name, DownloadFormat(ExportFormatAuto))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this one should do a pro-forma defer close(b).

@nfx nfx force-pushed the upload-download branch from f019518 to da42b50 Compare June 13, 2023 09:49
@nfx nfx merged commit 04377a7 into main Jun 13, 2023
@nfx nfx deleted the upload-download branch June 13, 2023 10:05
pietern added a commit to databricks/cli that referenced this pull request Jun 14, 2023
## Changes

This pulls in the Files API work from
databricks/databricks-sdk-go#423.

## Tests

Integration tests pass.
@nfx nfx mentioned this pull request Jun 15, 2023
nfx added a commit that referenced this pull request Jun 15, 2023
# Version changelog

## 0.10.0

* Added log level support to SimpleLogger. Default logger now prints
only `INFO` level messages. To replicate more verbose behavior from the
previous versions, set the `DEBUG` level in `SimpleLogger` explicitly
([#426](#426)).
* Added `Upload` and `Download` methods where applicable
([#423](#423)).
* Added auth tests for a databrickscfg file with an empty DEFAULT
profile
([#496](#496)).
* Added more integration tests and examples
([#503](#503)).
* Added preview level functions for services and methods to OpenAPI code
([#497](#497)).
* Added IsAllRequiredFieldsPrimitive helper function
([#404](#404)).
* Added isCrudCreate method
([#416](#416)).
* Added retries for getting refreshable token
([#500](#500)).
* Added type update when entity fields are updated
([#411](#411)).
* Don't panic but return error from NewAccountClient
([#422](#422)).
* Error when opening a DBFS directory for reading
([#415](#415)).
* Fixed cleanup logic in SQL warehouse integration test
([#400](#400)).
* Fixed error handling in cluster utility function
([#399](#399)).
* Fixed example in README.md
([#494](#494)).
* Fixed nondeterminism in workspace filesystem integration test
([#401](#401)).
* Improve command execution interface
([#410](#410)).
* Introduce waiters as top-level methods
([#408](#408)).
* Regenerate examples if configured
([#518](#518)).
* Respect limit field passed in ListRequests
([#407](#407)).
* Updated from OpenAPI spec
([#412](#412),
[#413](#413),
[#421](#421),
[#519](#519),
[#424](#424)).
* Updated API test template with page tokens
([#417](#417)).
* Use constants instead of hardcoding strings
([#402](#402)).
* Use `x-databricks-is-accounts` flag to determine whether a service is
an account level service
([#420](#420)).

API Changes:

github.com/databricks/databricks-sdk-go/service/serving
- (*ServingEndpointsAPI).Create: changed from func(context.Context,
CreateServingEndpoint) (*ServingEndpointDetailed, error) to
func(context.Context, CreateServingEndpoint)
(*WaitGetServingEndpointNotUpdating[ServingEndpointDetailed], error)
  - (*ServingEndpointsAPI).List: removed
- (*ServingEndpointsAPI).UpdateConfig: changed from
func(context.Context, EndpointCoreConfigInput)
(*ServingEndpointDetailed, error) to func(context.Context,
EndpointCoreConfigInput)
(*WaitGetServingEndpointNotUpdating[ServingEndpointDetailed], error)
  Compatible changes:
  - (*ServingEndpointsAPI).ListAll: added
  - (*ServingEndpointsAPI).WaitGetServingEndpointNotUpdating: added
  - WaitGetServingEndpointNotUpdating: added

github.com/databricks/databricks-sdk-go
  - WorkspaceClient.CommandExecutor: removed
  - WorkspaceClient.CommandExecution: added
  - WorkspaceClient.Connections: added
  - WorkspaceClient.Files: added
  - WorkspaceClient.SystemSchemas: added

github.com/databricks/databricks-sdk-go/service/sql
- (*WarehousesAPI).Create: changed from func(context.Context,
CreateWarehouseRequest) (*CreateWarehouseResponse, error) to
func(context.Context, CreateWarehouseRequest)
(*WaitGetWarehouseRunning[CreateWarehouseResponse], error)
  - (*WarehousesAPI).DeleteAndWait: removed
  - (*WarehousesAPI).DeleteByIdAndWait: removed
- (*WarehousesAPI).Edit: changed from func(context.Context,
EditWarehouseRequest) error to func(context.Context,
EditWarehouseRequest) (*WaitGetWarehouseRunning[any], error)
  - (*WarehousesAPI).GetAndWait: removed
  - (*WarehousesAPI).GetByIdAndWait: removed
- (*WarehousesAPI).Start: changed from func(context.Context,
StartRequest) error to func(context.Context, StartRequest)
(*WaitGetWarehouseRunning[any], error)
- (*WarehousesAPI).Stop: changed from func(context.Context, StopRequest)
error to func(context.Context, StopRequest)
(*WaitGetWarehouseStopped[any], error)
  - (*StatementExecutionAPI).ExecuteAndWait: added
  - (*WarehousesAPI).WaitGetWarehouseRunning: added
  - (*WarehousesAPI).WaitGetWarehouseStopped: added
  - FormatCsv: added
  - WaitGetWarehouseRunning: added
  - WaitGetWarehouseStopped: added

github.com/databricks/databricks-sdk-go/service/jobs
- (*JobsAPI).CancelRun: changed from func(context.Context, CancelRun)
error to func(context.Context, CancelRun)
(*WaitGetRunJobTerminatedOrSkipped[any], error)
  - (*JobsAPI).GetRunAndWait: removed
- (*JobsAPI).RepairRun: changed from func(context.Context, RepairRun)
(*RepairRunResponse, error) to func(context.Context, RepairRun)
(*WaitGetRunJobTerminatedOrSkipped[RepairRunResponse], error)
- (*JobsAPI).RunNow: changed from func(context.Context, RunNow)
(*RunNowResponse, error) to func(context.Context, RunNow)
(*WaitGetRunJobTerminatedOrSkipped[RunNowResponse], error)
- (*JobsAPI).Submit: changed from func(context.Context, SubmitRun)
(*SubmitRunResponse, error) to func(context.Context, SubmitRun)
(*WaitGetRunJobTerminatedOrSkipped[SubmitRunResponse], error)
  - (*JobsAPI).WaitGetRunJobTerminatedOrSkipped: added
  - CreateJob.RunAs: added
  - JobRunAs: added
  - JobSettings.RunAs: added
  - WaitGetRunJobTerminatedOrSkipped: added

github.com/databricks/databricks-sdk-go/service/compute
- (*ClustersAPI).Create: changed from func(context.Context,
CreateCluster) (*CreateClusterResponse, error) to func(context.Context,
CreateCluster) (*WaitGetClusterRunning[CreateClusterResponse], error)
- (*ClustersAPI).Delete: changed from func(context.Context,
DeleteCluster) error to func(context.Context, DeleteCluster)
(*WaitGetClusterTerminated[any], error)
- (*ClustersAPI).Edit: changed from func(context.Context, EditCluster)
error to func(context.Context, EditCluster)
(*WaitGetClusterRunning[any], error)
  - (*ClustersAPI).GetAndWait: removed
  - (*ClustersAPI).GetByClusterIdAndWait: removed
- (*ClustersAPI).Resize: changed from func(context.Context,
ResizeCluster) error to func(context.Context, ResizeCluster)
(*WaitGetClusterRunning[any], error)
- (*ClustersAPI).Restart: changed from func(context.Context,
RestartCluster) error to func(context.Context, RestartCluster)
(*WaitGetClusterRunning[any], error)
- (*ClustersAPI).Start: changed from func(context.Context, StartCluster)
error to func(context.Context, StartCluster)
(*WaitGetClusterRunning[any], error)
- (*CommandExecutionAPI).Cancel: changed from func(context.Context,
CancelCommand) error to func(context.Context, CancelCommand)
(*WaitCommandStatusCommandExecutionCancelled[any], error)
- (*CommandExecutionAPI).Create: changed from func(context.Context,
CreateContext) (*Created, error) to func(context.Context, CreateContext)
(*WaitContextStatusCommandExecutionRunning[Created], error)
- (*CommandExecutionAPI).Execute: changed from func(context.Context,
Command) (*Created, error) to func(context.Context, Command)
(*WaitCommandStatusCommandExecutionFinishedOrError[Created], error)
  - (*ClustersAPI).WaitGetClusterRunning: added
  - (*ClustersAPI).WaitGetClusterTerminated: added
  - (*CommandExecutionAPI).Start: added
- (*CommandExecutionAPI).WaitCommandStatusCommandExecutionCancelled:
added
-
(*CommandExecutionAPI).WaitCommandStatusCommandExecutionFinishedOrError:
added
- (*CommandExecutionAPI).WaitContextStatusCommandExecutionRunning: added
  - BaseClusterInfo.DataSecurityMode: added
  - BaseClusterInfo.DockerImage: added
  - BaseClusterInfo.SingleUserName: added
  - ClusterAttributes.DataSecurityMode: added
  - ClusterAttributes.DockerImage: added
  - ClusterAttributes.SingleUserName: added
  - ClusterInfo.DockerImage: added
  - CommandExecutorV2: added
  - EditCluster.DataSecurityMode: added
  - EditCluster.DockerImage: added
  - EditCluster.SingleUserName: added
  - WaitCommandStatusCommandExecutionCancelled: added
  - WaitCommandStatusCommandExecutionFinishedOrError: added
  - WaitContextStatusCommandExecutionRunning: added
  - WaitGetClusterRunning: added
  - WaitGetClusterTerminated: added

github.com/databricks/databricks-sdk-go/service/pipelines
  - (*PipelinesAPI).GetAndWait: removed
  - (*PipelinesAPI).GetByPipelineIdAndWait: removed
- (*PipelinesAPI).Reset: changed from func(context.Context,
ResetRequest) error to func(context.Context, ResetRequest)
(*WaitGetPipelineRunning[any], error)
- (*PipelinesAPI).Stop: changed from func(context.Context, StopRequest)
error to func(context.Context, StopRequest) (*WaitGetPipelineIdle[any],
error)
  - (*PipelinesAPI).WaitGetPipelineIdle: added
  - (*PipelinesAPI).WaitGetPipelineRunning: added
  - WaitGetPipelineIdle: added
  - WaitGetPipelineRunning: added

github.com/databricks/databricks-sdk-go/service/sharing
  - (*ProvidersAPI).ListShares: removed
  - (*ProvidersAPI).ListSharesAll: added

github.com/databricks/databricks-sdk-go/service/workspace
  - ExportFormatAuto: removed
  - ExportRequest.DirectDownload: removed
  - Import.Format: changed from ExportFormat to ImportFormat
  - (*WorkspaceAPI).Download: added
  - (*WorkspaceAPI).ReadFile: added
  - (*WorkspaceAPI).Upload: added
  - (*WorkspaceAPI).WriteFile: added
  - DownloadFormat: added
  - DownloadOption: added
  - ImportFormat: added
  - ImportFormatAuto: added
  - ImportFormatDbc: added
  - ImportFormatHtml: added
  - ImportFormatJupyter: added
  - ImportFormatRMarkdown: added
  - ImportFormatSource: added
  - UploadFormat: added
  - UploadLanguage: added
  - UploadOption: added
  - UploadOverwrite: added

github.com/databricks/databricks-sdk-go/service/catalog
  - (*FunctionsAPI).List: removed
- (*MetastoresAPI).Assign: changed from func(context.Context,
CreateMetastoreAssignment) error to func(context.Context,
CreateMetastoreAssignment) error
- (*MetastoresAPI).Create: changed from func(context.Context,
CreateMetastore) (*MetastoreInfo, error) to func(context.Context,
CreateMetastore) (*MetastoreInfo, error)
- (*MetastoresAPI).Update: changed from func(context.Context,
UpdateMetastore) (*MetastoreInfo, error) to func(context.Context,
UpdateMetastore) (*MetastoreInfo, error)
- (*MetastoresAPI).UpdateAssignment: changed from func(context.Context,
UpdateMetastoreAssignment) error to func(context.Context,
UpdateMetastoreAssignment) error
- (*StorageCredentialsAPI).Create: changed from func(context.Context,
CreateStorageCredential) (*StorageCredentialInfo, error) to
func(context.Context, CreateStorageCredential) (*StorageCredentialInfo,
error)
- (*StorageCredentialsAPI).Update: changed from func(context.Context,
UpdateStorageCredential) (*StorageCredentialInfo, error) to
func(context.Context, UpdateStorageCredential) (*StorageCredentialInfo,
error)
  - (*TablesAPI).ListSummaries: removed
  - CreateMetastore.Name: removed
  - CreateMetastore.Region: removed
  - CreateMetastore.StorageRoot: removed
  - CreateMetastore: changed from CreateMetastore to CreateMetastore
  - CreateMetastoreAssignment.DefaultCatalogName: removed
- CreateMetastoreAssignment: changed from CreateMetastoreAssignment to
CreateMetastoreAssignment
  - CreateStorageCredential.AwsIamRole: removed
  - CreateStorageCredential.AzureServicePrincipal: removed
  - CreateStorageCredential.Comment: removed
  - CreateStorageCredential.GcpServiceAccountKey: removed
  - CreateStorageCredential.Name: removed
  - CreateStorageCredential.ReadOnly: removed
  - CreateStorageCredential.SkipValidation: removed
- CreateStorageCredential: changed from CreateStorageCredential to
CreateStorageCredential
  - GcpServiceAccountKey: removed
  - ListFunctionsResponse.Schemas: removed
  - MetastoreAssignment.WorkspaceId: changed from string to int64
  - StorageCredentialInfo.GcpServiceAccountKey: removed
  - UpdateMetastore.DeltaSharingOrganizationName: removed
  - UpdateMetastore.DeltaSharingRecipientTokenLifetimeInSeconds: removed
  - UpdateMetastore.DeltaSharingScope: removed
  - UpdateMetastore.Id: removed
  - UpdateMetastore.Name: removed
  - UpdateMetastore.Owner: removed
  - UpdateMetastore.PrivilegeModelVersion: removed
  - UpdateMetastore.StorageRootCredentialId: removed
  - UpdateMetastore: changed from UpdateMetastore to UpdateMetastore
  - UpdateMetastoreAssignment.DefaultCatalogName: removed
- UpdateMetastoreAssignment: changed from UpdateMetastoreAssignment to
UpdateMetastoreAssignment
  - UpdateStorageCredential.AwsIamRole: removed
  - UpdateStorageCredential.AzureServicePrincipal: removed
  - UpdateStorageCredential.Comment: removed
  - UpdateStorageCredential.Force: removed
  - UpdateStorageCredential.GcpServiceAccountKey: removed
  - UpdateStorageCredential.Owner: removed
  - UpdateStorageCredential.ReadOnly: removed
  - UpdateStorageCredential.SkipValidation: removed
- UpdateStorageCredential: changed from UpdateStorageCredential to
UpdateStorageCredential
  - ValidateStorageCredential.GcpServiceAccountKey: removed
  - (*FunctionsAPI).FunctionInfoNameToFullNameMap: added
  - (*FunctionsAPI).ListAll: added
  - (*TablesAPI).ListSummariesAll: added
  - AccountsCreateMetastore: added
  - AccountsCreateMetastoreAssignment: added
  - AccountsCreateStorageCredential: added
  - AccountsUpdateMetastore: added
  - AccountsUpdateMetastoreAssignment: added
  - AccountsUpdateStorageCredential: added
  - AzureManagedIdentity: added
  - ConnectionInfo: added
  - ConnectionType: added
  - ConnectionTypeDatabricks: added
  - ConnectionTypeMysql: added
  - ConnectionTypePostgresql: added
  - ConnectionTypeRedshift: added
  - ConnectionTypeSnowflake: added
  - ConnectionTypeSqldw: added
  - ConnectionTypeSqlserver: added
  - ConnectionsAPI: added
  - ConnectionsService: added
  - CreateConnection: added
  - CreateMetastore.MetastoreInfo: added
  - CreateMetastoreAssignment.MetastoreAssignment: added
  - CreateStorageCredential.CredentialInfo: added
  - CredentialType: added
  - CredentialTypeUsernamePassword: added
  - DatabricksGcpServiceAccountResponse: added
  - DeleteConnectionRequest: added
  - DisableRequest: added
  - GetConnectionRequest: added
  - ListConnectionsResponse: added
  - ListFunctionsResponse.Functions: added
  - ListSystemSchemasRequest: added
  - ListSystemSchemasResponse: added
  - NewConnections: added
  - NewSystemSchemas: added
  - OptionsKvPairs: added
  - PropertiesKvPairs: added
  - StorageCredentialInfo.AzureManagedIdentity: added
  - StorageCredentialInfo.DatabricksGcpServiceAccount: added
  - SystemSchemaInfo: added
  - SystemSchemaInfoState: added
  - SystemSchemaInfoStateDisableinitialized: added
  - SystemSchemaInfoStateEnablecompleted: added
  - SystemSchemaInfoStateEnableinitialized: added
  - SystemSchemaInfoStateUnavailable: added
  - SystemSchemasAPI: added
  - SystemSchemasService: added
  - UpdateConnection: added
  - UpdateMetastore.MetastoreInfo: added
  - UpdateMetastoreAssignment.MetastoreAssignment: added
  - UpdateStorageCredential.CredentialInfo: added
  - ValidateStorageCredential.AzureManagedIdentity: added
  - ValidateStorageCredential.DatabricksGcpServiceAccount: added

github.com/databricks/databricks-sdk-go/service/ml
  - GetModelResponse.RegisteredModel: removed
  - GetModelResponse.RegisteredModelDatabricks: added

github.com/databricks/databricks-sdk-go/service/files
  - FilesAPI: added
  - FilesService: added
  - NewFiles: added

github.com/databricks/databricks-sdk-go/logger
  - SimpleLogger.Level: added

github.com/databricks/databricks-sdk-go/service/provisioning
- (*WorkspacesAPI).Create: changed from func(context.Context,
CreateWorkspaceRequest) (*Workspace, error) to func(context.Context,
CreateWorkspaceRequest) (*WaitGetWorkspaceRunning[Workspace], error)
- (*WorkspacesAPI).Update: changed from func(context.Context,
UpdateWorkspaceRequest) error to func(context.Context,
UpdateWorkspaceRequest) (*WaitGetWorkspaceRunning[any], error)
  - (*WorkspacesAPI).WaitGetWorkspaceRunning: added
  - WaitGetWorkspaceRunning: added

github.com/databricks/databricks-sdk-go/service/iam
  - AccountAccessControlAPI: added
  - AccountAccessControlProxyAPI: added
  - AccountAccessControlProxyService: added
  - AccountAccessControlService: added
  - GetAssignableRolesForResourceRequest: added
  - GetAssignableRolesForResourceResponse: added
  - GetRuleSetRequest: added
  - GrantRule: added
  - NewAccountAccessControl: added
  - NewAccountAccessControlProxy: added
  - RuleSetResponse: added
  - RuleSetUpdateRequest: added
  - UpdateRuleSetRequest: added

Dependency updates:

* Bump github.com/stretchr/testify from 1.8.3 to 1.8.4
([#406](#406)).
* Bump golang.org/x/mod from 0.10.0 to 0.11.0
([#515](#515)).
* Bump golang.org/x/oauth2 from 0.8.0 to 0.9.0
([#498](#498)).
* Bump google.golang.org/api from 0.123.0 to 0.127.0
([#405](#405),
[#425](#425),
[#429](#429)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants