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.

Sign up for GitHub

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

Jump to bottom

Add more readability for errors #129

Merged
merged 33 commits into from
Jun 25, 2020
Merged

Add more readability for errors #129

merged 33 commits into from
Jun 25, 2020

Conversation

nfx
Copy link
Contributor

@nfx nfx commented Jun 19, 2020 •
edited
Loading

  • Add resource wait retry for workspace create
  • Made vscode integration testing simpler
  • added read support for username/password for config files
  • Made errors concise and explainable

@nfx nfx requested a review from stikkireddy June 19, 2020 15:28
@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2020 •
edited
Loading

Codecov Report

Merging #129 into master will increase coverage by 0.21%.
The diff coverage is 34.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
+ Coverage   48.02%   48.24%   +0.21%     
==========================================
  Files          58       58              
  Lines        7219     7245      +26     
==========================================
+ Hits         3467     3495      +28     
+ Misses       3690     3681       -9     
- Partials       62       69       +7
Flag Coverage Δ
#client 74.76% <62.12%> (-0.57%) ⬇️
#provider 40.64% <16.34%> (+0.17%) ⬆️
Impacted Files Coverage Δ
client/service/secret_scopes.go 75.67% <0.00%> (-9.18%) ⬇️
client/service/secrets.go 80.00% <0.00%> (-7.81%) ⬇️
databricks/azure_auth.go 0.00% <0.00%> (ø)
...ricks/resource_databricks_azure_adls_gen1_mount.go 37.33% <0.00%> (ø)
...ricks/resource_databricks_azure_adls_gen2_mount.go 37.10% <0.00%> (ø)
databricks/resource_databricks_azure_blob_mount.go 36.23% <0.00%> (ø)
databricks/resource_databricks_cluster_policy.go 35.61% <0.00%> (-2.07%) ⬇️
databricks/resource_databricks_dbfs_file.go 35.64% <0.00%> (+1.02%) ⬆️
databricks/resource_databricks_group.go 26.08% <0.00%> (ø)
...icks/resource_databricks_group_instance_profile.go 25.71% <0.00%> (ø)
... and 22 more

@nfx nfx mentioned this pull request Jun 21, 2020
Merged
stikkireddy
stikkireddy reviewed Jun 21, 2020
View reviewed changes
docs/index.md Outdated Show resolved Hide resolved
nfx and others added 17 commits June 22, 2020 22:15
@nfx
Add resource wait retry for workspace create
97361c6
@nfx
Made vscode integration testing simpler
285995a
@nfx
added read support for username/password for config files
9b09084
@nfx
Made errors concise and explainable
4b931c9
@nfx
Fix formatting issue
0ea8c51
@nfx
cleaned up token request structs
8b17019
@nfx
make fmt
a64ba56
@nfx
Added some documentation
94ac0e3
@nfx
keying composite literals
0f68309
@nfx
Add missing resource check in resourceClusterPolicyRead
a076e0d
@nfx
Apply review comments
e631e3c
@nfx
Merge branch 'fixes' of github.com:databrickslabs/terraform-provider-…
2c063c6 
…databricks into fixes
@nfx
More correct implementation of 404-check
bbb9a7e
@nfx
Make README more user-friendly
05d59ec
@nfx
More links
7e8f0ff
@stikkireddy
added integration test to verify that all apis can either handle 404s…
3a2d754 
… or check the error message to verify resource is missing; added travis buddy integration; added unit test for handling errors; deprecated dbfs file sync; doc fix for scim user;
@stikkireddy
added skip for testMissingWorkspaceResources to run only if TF_ACC is…
a8747e0 
… set so it runs with both aws and azure integration tests
@stikkireddy
Merge branches 'fixes' and 'master' of github.com:databrickslabs/terr…
94580cb 
…aform-provider-databricks into fixes

� Conflicts:
�	databricks/azure_auth.go
@stikkireddy
cleaned up makefile
7cdaa55
@stikkireddy
refactored tokenexpirytime to the api client config so the client is …
742ded6 
…aware of expiry time of token as metadata; added getAndAssetEnv for generating the dbapi client for testing missing errors
@TravisBuddy
Copy link

Hey @nfx,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 155e26f0-b68c-11ea-aaf6-456b40a357ff

@TravisBuddy
Copy link

Hey @nfx,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 9fac8190-b690-11ea-aaf6-456b40a357ff

@stikkireddy stikkireddy mentioned this pull request Jun 25, 2020
Merged
stikkireddy
stikkireddy reviewed Jun 25, 2020
View reviewed changes
docs/index.md
@@ -6,59 +6,73 @@ description: |-
Terraform provider databricks.
---

# Databricks Provider
Copy link
Contributor

Choose a reason for hiding this comment

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

@nfx Headers should align to the common naming convention provided by Hashicorp which are the conventions that the other official providers follow. Guide can be found here: https://www.terraform.io/docs/registry/providers/docs.html#headers. I am reverting it to follow that guide and examples can be found here:
Old Site:

  1. https://www.terraform.io/docs/providers/aws/index.html
  2. https://www.terraform.io/docs/providers/azurerm/index.html

New Registry Docs:

  1. https://registry.terraform.io/providers/hashicorp/aws/latest/docs#authentication
  2. https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs

This gives the user similar experience when using the docs.

We should try to use the main headers as it is used to determine the nav on the registry on the right hand side of the page. All H2 or "##" show up on that nav bar so too many makes it confusing. I am going to follow the pattern on the AWS provider as it is very simple nav and you can generate links to specific areas if we need to send links for the H3 blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--- in the beginning of the file is equal to H1, just in case, but comment noted

@TravisBuddy
Copy link

Hey @nfx,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 3783c4b0-b69b-11ea-aaf6-456b40a357ff

@TravisBuddy
Copy link

Hey @nfx,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 594857b0-b69f-11ea-aaf6-456b40a357ff

@TravisBuddy
Copy link

Hey @nfx,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: a4519a70-b6a2-11ea-aaf6-456b40a357ff

@stikkireddy stikkireddy merged commit 6f50437 into master Jun 25, 2020
@nfx nfx deleted the fixes branch June 25, 2020 08:57
@nfx nfx added the azure Occurring on Azure cloud label Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stikkireddy stikkireddy stikkireddy approved these changes

Assignees
No one assigned
Labels
azure Occurring on Azure cloud
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nfx @codecov-commenter @TravisBuddy @stikkireddy