Skip to content
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

[Internal] Arrange service in a per package sdk approach #1123

Open
wants to merge 24 commits into
base: dev/sdk-mod
Choose a base branch
from

Conversation

parthban-db
Copy link
Contributor

@parthban-db parthban-db commented Jan 14, 2025

What changes are proposed in this pull request?

This PR moves all the service packages into separate Go modules and isolates them from each other. It also removes the workspace and account clients, which are to be replaced by the per-package client. It removes all the integration tests and examples as they use existing clients so we need to revisit these in the future.

How is this tested?

Existing unit tests. But we need to revisit the integration tests.

@parthban-db parthban-db changed the base branch from main to dev/sdk-mod January 14, 2025 13:12
// TrimLeadingWhitespace removes leading whitespace, so that Python code blocks
// that are embedded into Go code still could be interpreted properly.
// TODO: for note this is from the compute package
func TrimLeadingWhitespace(commandStr string) (newCommand string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied from the compute package to break dependency

@@ -181,48 +188,15 @@ func (a *servingEndpointsImpl) UpdatePermissions(ctx context.Context, request Se

// unexported type that holds implementations of just ServingEndpointsDataPlane API methods
type servingEndpointsDataPlaneImpl struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to fix this.

@@ -1,94 +0,0 @@
// Code generated from OpenAPI specs by Databricks SDK Generator. DO NOT EDIT.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need an interface, right?

@@ -1,49 +0,0 @@
// Databricks File System (DBFS) API
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need this?

@@ -1,164 +0,0 @@
package compute
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to implement wait for state first.

permissions PermissionsInterface
}

func NewPermissionsClientFromConfig(c ...*config.Config) (*PermissionsClient, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a copy of how we make workspace client.

Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-go

Inputs:

  • PR number: 1123
  • Commit SHA: 2b18d8e92d2a76a72868bc22ba0b9eb974309803

Checks will be approved automatically on success.

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.

1 participant