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

Refactoring of bitbucket provider with better error support and general improvments #13390

Merged
merged 1 commit into from
Apr 6, 2017

Conversation

cwood
Copy link
Contributor

@cwood cwood commented Apr 5, 2017

Also doesnt use decode/encode anymore since those methods are more
intended for streams. So this goes to the more standed marshal/unmarshal
of data.

This also adds better error support and will try to give the user
better errors from the api if it can. Or any issues with the bitbucket
service.

Also doesnt use decode/encode anymore since those methods are more
intended for streams. So this goes to the more standed marshal/unmarshal
of data.

This also adds better error support and will try to give the user
better errors from the api if it can. Or any issues with the bitbucket
service.
@cwood
Copy link
Contributor Author

cwood commented Apr 5, 2017

I have run the acceptence test and they work. Before the tests were failing on EOF cause of using decode.

=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccBitbucketDefaultReviewers_basic
--- PASS: TestAccBitbucketDefaultReviewers_basic (4.37s)
=== RUN   TestAccBitbucketHook_basic
--- PASS: TestAccBitbucketHook_basic (5.60s)
=== RUN   TestAccBitbucketRepository_basic
--- PASS: TestAccBitbucketRepository_basic (2.79s)
PASS
ok      github.com/cwood/terraform/builtin/providers/bitbucket  12.778s

@apparentlymart
Copy link
Contributor

Hi @cwood!

Just to make sure I understand what we have here... does this address the question raised on StackOverflow, either by making that error go away or by returning a more helpful error? I think I remember you saying that it was an authentication error causing that failure, so perhaps this change just makes Terraform return a more specific error message in that situation?

@cwood
Copy link
Contributor Author

cwood commented Apr 6, 2017

This solves that issue by having better error messages. Most of their issues were fine even when authenticated. From what I remember using decode/encode is meant for streams of data so it can be a issue with it.

Copy link
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

This all looks good to me!

I don't know the BitBucket API at all, so I'm just trusting that you know what you're doing there 😀 but overall this approach looks good to me and I'm excited for better error messages.

@cwood cwood merged commit c88b19e into hashicorp:master Apr 6, 2017
@ghost
Copy link

ghost commented Apr 14, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants