Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexott
alexott committed Aug 13, 2021
1 parent a7a9486 commit 890e888
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 137 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
| [databricks_obo_token](docs/resources/obo_token.md)
| [databricks_permissions](docs/resources/permissions.md)
| [databricks_pipeline](docs/resources/pipeline.md)
| [databricks_repo](docs/resources/repo.md)
| [databricks_secret](docs/resources/secret.md)
| [databricks_secret_acl](docs/resources/secret_acl.md)
| [databricks_secret_scope](docs/resources/secret_scope.md)
Expand Down
1 change: 1 addition & 0 deletions docs/index.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Compute resources
* Speedup job & cluster startup with [databricks_instance_pool](resources/instance_pool.md)
* Customize clusters with [databricks_global_init_script](resources/global_init_script.md)
* Manage few [databricks_notebook](resources/notebook.md), and even [list them](data-sources/notebook_paths.md)
* Manage [databricks_repo](resources/repo.md)

Storage
* Manage JAR, Wheel & Egg libraries through [databricks_dbfs_file](resources/dbfs_file.md)
Expand Down
Binary file modified docs/resources.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion docs/resources/permissions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ resource "databricks_repo" "this" {
url = "https://github.com/user/demo.git"
}
resource "databricks_permissions" "notebook_usage" {
resource "databricks_permissions" "repo_usage" {
repo_id = databricks_repo.this.id
access_control {
Expand Down
8 changes: 4 additions & 4 deletions exporter/exporter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -940,12 +940,12 @@ func TestEitherString(t *testing.T) {
}

func TestImportingRepos(t *testing.T) {
resp := workspace.ReposResponse{
Id: 121232342,
resp := workspace.ReposInformation{
ID: 121232342,
Url: "https://github.com/user/test.git",
Provider: "gitHub",
Path: "/Repos/user@domain/test",
HeadCommitId: "1124323423abc23424",
HeadCommitID: "1124323423abc23424",
Branch: "releases",
}

Expand All @@ -956,7 +956,7 @@ func TestImportingRepos(t *testing.T) {
Method: "GET",
Resource: "/api/2.0/repos?",
Response: workspace.ReposListResponse{
Repos: []workspace.ReposResponse{
Repos: []workspace.ReposInformation{
resp,
},
},
Expand Down
2 changes: 1 addition & 1 deletion exporter/importables.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ var resourcesMap map[string]importable = map[string]importable{
if repo.Url != "" {
ic.Emit(&resource{
Resource: "databricks_repo",
ID: fmt.Sprintf("%d", repo.Id),
ID: fmt.Sprintf("%d", repo.ID),
})
}
log.Printf("[INFO] Scanned %d of %d repos", offset+1, len(repoList))
Expand Down
164 changes: 75 additions & 89 deletions workspace/resource_repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/databrickslabs/terraform-provider-databricks/common"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
)

// ReposAPI exposes the Repos API
Expand All @@ -21,14 +22,19 @@ func NewReposAPI(ctx context.Context, m interface{}) ReposAPI {
return ReposAPI{m.(*common.DatabricksClient), ctx}
}

// ReposResponse provides information about given repository
type ReposResponse struct {
Id int64 `json:"id"`
// ReposInformation provides information about given repository
type ReposInformation struct {
ID int64 `json:"id"`
Url string `json:"url"`
Provider string `json:"provider"`
Path string `json:"path"`
Branch string `json:"branch"`
HeadCommitId string `json:"head_commit_id"`
Provider string `json:"provider,omitempty" tf:"computed,alias:git_provider"`
Path string `json:"path,omitempty" tf:"computed"`
Branch string `json:"branch,omitempty" tf:"computed"`
HeadCommitID string `json:"head_commit_id,omitempty" tf:"computed,alias:commit_hash"`
}

// ID returns job id as string
func (r ReposInformation) RepoID() string {
return fmt.Sprintf("%d", r.ID)
}

type createRequest struct {
Expand All @@ -37,8 +43,15 @@ type createRequest struct {
Path string `json:"path,omitempty"`
}

func (a ReposAPI) Create(r createRequest) (ReposResponse, error) {
var resp ReposResponse
func (a ReposAPI) Create(r createRequest) (ReposInformation, error) {
var resp ReposInformation
if r.Provider == "" { // trying to infer Git Provider from the URL
r.Provider = getProviderFromUrl(r.Url)
}
if r.Provider == "" {
return resp, fmt.Errorf("git_provider isn't specified and we can't detect provider from URL")
}

err := a.client.Post(a.context, "/repos", r, &resp)
return resp, err
}
Expand All @@ -48,26 +61,38 @@ func (a ReposAPI) Delete(id string) error {
}

func (a ReposAPI) Update(id string, r map[string]string) error {
if len(r) == 0 {
return nil
}
// TODO: update may change ONE OF (url AND provider (optional)), (path), or (branch OR tag).
// for URL/provider force re-create as there are limits on what could be done for changing URL/provider
if path, ok := r["path"]; ok {
err := a.client.Patch(a.context, fmt.Sprintf("/repos/%s", id), map[string]string{"path": path})
if err != nil {
return err
}
delete(r, "path")
}
return a.client.Patch(a.context, fmt.Sprintf("/repos/%s", id), r)
}

func (a ReposAPI) Read(id string) (ReposResponse, error) {
var resp ReposResponse
func (a ReposAPI) Read(id string) (ReposInformation, error) {
var resp ReposInformation
err := a.client.Get(a.context, fmt.Sprintf("/repos/%s", id), nil, &resp)
return resp, err
}

type ReposListResponse struct {
NextPageToken string `json:"next_page_token,omitempty"`
Repos []ReposResponse `json:"repos"`
NextPageToken string `json:"next_page_token,omitempty"`
Repos []ReposInformation `json:"repos"`
}

func (a ReposAPI) List(prefix string) ([]ReposResponse, error) {
func (a ReposAPI) List(prefix string) ([]ReposInformation, error) {
req := map[string]string{}
if prefix != "" {
req["path_prefix"] = prefix
}
reposList := []ReposResponse{}
reposList := []ReposInformation{}
for {
var resp ReposListResponse
err := a.client.Get(a.context, "/repos", req, &resp)
Expand All @@ -83,7 +108,7 @@ func (a ReposAPI) List(prefix string) ([]ReposResponse, error) {
return reposList, nil
}

func (a ReposAPI) ListAll() ([]ReposResponse, error) {
func (a ReposAPI) ListAll() ([]ReposInformation, error) {
return a.List("")
}

Expand All @@ -104,110 +129,71 @@ func getProviderFromUrl(uri string) string {
}

func ResourceRepo() *schema.Resource {
s := map[string]*schema.Schema{
"url": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"git_provider": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
return strings.EqualFold(old, new)
},
},
"path": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true, // TODO: remove after the Update API will support changing the path
},
"branch": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ConflictsWith: []string{"tag"},
},
"tag": {
s := common.StructToSchema(ReposInformation{}, func(s map[string]*schema.Schema) map[string]*schema.Schema {
s["url"].ForceNew = true
s["url"].ValidateFunc = validation.IsURLWithScheme([]string{"https", "http"})
s["git_provider"].ForceNew = true
s["git_provider"].DiffSuppressFunc = func(k, old, new string, d *schema.ResourceData) bool {
return strings.EqualFold(old, new)
}
s["path"].ForceNew = true // TODO: remove after the Update API will support changing the path
s["branch"].ConflictsWith = []string{"tag"}
s["branch"].ValidateFunc = validation.StringIsNotWhiteSpace

s["tag"] = &schema.Schema{
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"branch"},
},
"commit_hash": {
Type: schema.TypeString,
Computed: true,
},
}
ValidateFunc: validation.StringIsNotWhiteSpace,
}

delete(s, "id")
return s
})

return common.Resource{
Schema: s,
SchemaVersion: 1,
Create: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
reposAPI := NewReposAPI(ctx, c)
url := d.Get("url").(string)
provider := d.Get("git_provider").(string)
if provider == "" { // trying to infer Git Provider from the URL
provider = getProviderFromUrl(url)
}
if provider == "" {
return fmt.Errorf("git_provider isn't specified and we can't detect provider from URL")
}
req := createRequest{Path: d.Get("path").(string), Provider: provider, Url: url}
req := createRequest{Path: d.Get("path").(string), Provider: d.Get("git_provider").(string), Url: d.Get("url").(string)}
resp, err := reposAPI.Create(req)
if err != nil {
return err
}
d.SetId(fmt.Sprintf("%d", resp.Id))
d.SetId(resp.RepoID())
branch := d.Get("branch").(string)
tag := d.Get("tag").(string)
updateReq := map[string]string{}
if tag != "" {
d.Set("branch", "")
return reposAPI.Update(d.Id(), map[string]string{"tag": tag})
updateReq["tag"] = tag
} else if branch != "" && branch != resp.Branch {
return reposAPI.Update(d.Id(), map[string]string{"branch": branch})
updateReq["branch"] = branch
}

return nil
return reposAPI.Update(d.Id(), updateReq)
},
Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
reposAPI := NewReposAPI(ctx, c)
resp, err := reposAPI.Read(d.Id())
if err != nil {
return err
}
d.SetId(fmt.Sprintf("%d", resp.Id))
d.Set("url", resp.Url)
d.Set("path", resp.Path)
d.Set("git_provider", resp.Provider)
d.Set("commit_hash", resp.HeadCommitId)
if d.Get("branch").(string) == "" {
d.Set("branch", resp.Branch)
}
return nil
return common.StructToData(resp, s, d)
},
Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
// TODO: update may change ONE OF (url AND provider (optional)), (path), or (branch OR tag).
// for URL/provider force re-create as there are limits on what could be done for changing URL/provider
reposAPI := NewReposAPI(ctx, c)
req := map[string]string{}
// Not working yet, wait until API is ready
// if d.HasChange("path") {
// err := reposAPI.Update(d.Id(), map[string]string{"path": d.Get("path").(string)})
// if err != nil {
// return err
// }
// req["path"] = d.Get("path").(string)
// }
if d.HasChange("branch") || d.HasChange("tag") {
branch := d.Get("branch").(string)
tag := d.Get("tag").(string)
if tag != "" {
d.Set("branch", "")
return reposAPI.Update(d.Id(), map[string]string{"tag": tag})
}
return reposAPI.Update(d.Id(), map[string]string{"branch": branch})
if d.HasChange("tag") {
req["tag"] = d.Get("tag").(string)
d.Set("branch", "")
} else if d.HasChange("branch") {
req["branch"] = d.Get("branch").(string)
}
return nil
return reposAPI.Update(d.Id(), req)
},
Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
return NewReposAPI(ctx, c).Delete(d.Id())
Expand Down
Loading

0 comments on commit 890e888

Please sign in to comment.