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

Adding ability to add and update contact groups using the terraform statuscake provider #7115

Merged
merged 5 commits into from
Jan 17, 2017

Conversation

ldjohnson
Copy link

Currently, it is not possible to add/update contact groups using the Statuscake provider. This update adds the ability to add/update contact groups on Statuscake tests. This update adds a new element to the statuscake resource called 'contact_id'. It should be set to the id of the contact group you want to add.

I also updated the DreamItGetIt/statuscake/tests.go file a querystring was missing from the ContactID item in the Test struct. I have already submitted a pull request to that team, but I wanted to let you know anyway as I am unsure of how to update depedencies like that being that is my first contribution to an open source project.

Lastly, I added a new acceptance test precheck for two new environment variables (CONTACT_GROUP and ALT_CONTACT_GROUP). They are only needed for contact group acceptance tests I added and should be set to the id of the contact group you want to add and the contact group you want update respectively. I did this because each ID is unique and would need to be configured before the new tests would run successfully.

It also for this reason that created to new acceptance tests instead of modifying existing ones. This way, the new variables won't impact acceptance tests for users who are not using them.

I updated the documentation to reflect the new element as well. Anyway, please take a look and let me know if you have any questions or concerns.

@stack72
Copy link
Contributor

stack72 commented Jun 10, 2016

Hi @ldjohnson

Thanks for the PR here. I am going to get in contact with my old team about this dependency so we can get the upstream updated rather than manually changing the vendor folder

Stay tuned here

P.

@stack72 stack72 self-assigned this Jun 10, 2016
@stack72 stack72 added enhancement provider/statuscake waiting-response An issue/pull request is waiting for a response from the community labels Jun 10, 2016
@ldjohnson
Copy link
Author

Paul,

My pleasure. If there is anything else you need or if you have any
questions or concerns, please let me know.

On Fri, Jun 10, 2016 at 1:42 PM, Paul Stack notifications@github.com
wrote:

Hi @ldjohnson https://github.com/ldjohnson

Thanks for the PR here. I am going to get in contact with my old team
about this dependency so we can get the upstream updated rather than
manually changing the vendor folder

Stay tuned here

P.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7115 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AS3Ocu4M34iwNhvzywRnjoRHchSFqzYEks5qKaISgaJpZM4IzKr5
.

Regards,

Lee Johnson
Junior Software Developer
iJoin Solutions, LLC

@ldjohnson
Copy link
Author

All,

On a side note, I did sumbit a pull request to the DreamItGetIt/statuscake
repo with the change I mentioned in my pull request. Just wanted to
mention that in case I forgot to.

Please let me know if you have any questions or concerns.

On Fri, Jun 10, 2016 at 1:58 PM, Lee Johnson leejohnson@ijoinsolutions.com
wrote:

Paul,

My pleasure. If there is anything else you need or if you have any
questions or concerns, please let me know.

On Fri, Jun 10, 2016 at 1:42 PM, Paul Stack notifications@github.com
wrote:

Hi @ldjohnson https://github.com/ldjohnson

Thanks for the PR here. I am going to get in contact with my old team
about this dependency so we can get the upstream updated rather than
manually changing the vendor folder

Stay tuned here

P.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7115 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AS3Ocu4M34iwNhvzywRnjoRHchSFqzYEks5qKaISgaJpZM4IzKr5
.

Regards,

Lee Johnson
Junior Software Developer
iJoin Solutions, LLC

Regards,

Lee Johnson
Junior Software Developer
iJoin Solutions, LLC

@stack72
Copy link
Contributor

stack72 commented Jun 10, 2016

@ldjohnson I just saw that - this is why i will nudge them :)

@ldjohnson
Copy link
Author

Hello all,

I just wanted to checking on the status of this pull request. I there is anything still needed, please let me know. Thanks!

@stack72 stack72 removed the waiting-response An issue/pull request is waiting for a response from the community label Sep 3, 2016
@ldjohnson
Copy link
Author

Hello All,

I just wanted to check up on the status of this PR since I didn't hear anything back the last time. It looks like my PR on the underlying statuscake go library was accepted and merged in, so I wanted to check in:
DreamItGetIT/statuscake#4

If anything is still needed or missing for this PR, please let me know. Thanks!

@stack72
Copy link
Contributor

stack72 commented Nov 22, 2016

Hi @ldjohnson

I just re-vendored the dependencies rather than manually changing them. Will merge and test in a sec

P.

@stack72
Copy link
Contributor

stack72 commented Nov 22, 2016

Hi @ldjohnson

So I made a follow up commit to this (contact_id was added twice in the schema + the re-vendoring thing)

The tests look as follows:

% make testacc TEST=./builtin/providers/statuscake                                             2 ↵ ✹
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/11/22 14:04:10 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/statuscake -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccStatusCake_basic
--- PASS: TestAccStatusCake_basic (2.45s)
=== RUN   TestAccStatusCake_withUpdate
--- PASS: TestAccStatusCake_withUpdate (3.51s)
=== RUN   TestAccStatusCake_contactGroup_basic
--- FAIL: TestAccStatusCake_contactGroup_basic (0.00s)
	resource_statuscaketest_test.go:18: CONTACT_GROUP must be set for contact group acceptance tests
=== RUN   TestAccStatusCake_contactGroup_withUpdate
--- FAIL: TestAccStatusCake_contactGroup_withUpdate (0.00s)
	resource_statuscaketest_test.go:18: CONTACT_GROUP must be set for contact group acceptance tests
FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/statuscake	5.973s
make: *** [testacc] Error 1

What do I need to do here?

P.

@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Nov 22, 2016
@ldjohnson
Copy link
Author

Hmmm....not sure what the issue could be. I'll look into this and let you know what I find. Thanks for letting me know about this.

@ldjohnson
Copy link
Author

ldjohnson commented Nov 29, 2016

After looking into this, I found the cause of the issue that you ran into. When I first created the PR, I made the assumption that the contact ID had to be valid (that is, a contact with that Id had to already exist). To that end I required that the contact Id be set as an environment variable. I checked this prior to running the two new tests that I added. This check was failing because the env variables had not been set.

After looking at what was committed to master and see what was added in the follow up commit, I removed the two new tests (leaving only the existing basic and update tests) and removed the check for the env variables since they would not be needed anymore. I also removed the import for 'os' since it was no longer being used (the tests I removed were the only things using it).

Lastly, it appears that Statuscake changed the URL for their api. The current URL in the DreamItGetIt library results in a 301 removed permanently error and causes the tests to fail. I'll be pushing up a PR for that shortly to that project. As soon as I have, I'll post a link to that PR here.

This PR should be ready for testing and review and should be up-to-date with master as well. If you have any questions or concerns, please let me know. Thanks!

--Lee

@ldjohnson
Copy link
Author

ldjohnson commented Nov 29, 2016

I have submitted a PR to the DreamItGetIT/statuscake project to update statuscake's api url. That PR can be found here:

DreamItGetIT/statuscake#6

This PR will need to be merged in and added to the DreamItGetIT/statuscake library in order for the acceptance tests to pass. Without this update, the tests will fail as follows:

make testacc TEST=./builtin/providers/statuscake
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/11/29 16:47:29 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/statuscake -v -timeout 120m
=== RUN TestProvider
--- PASS: TestProvider (0.00s)
=== RUN TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN TestAccStatusCake_basic
--- FAIL: TestAccStatusCake_basic (0.93s)
testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:
* statuscake_test.google: Error creating StatusCake Test: HTTP error: 301 - 301 Moved Permanently
=== RUN TestAccStatusCake_withUpdate
--- FAIL: TestAccStatusCake_withUpdate (0.67s)
testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:
* statuscake_test.google: Error creating StatusCake Test: HTTP error: 301 - 301 Moved Permanently
FAIL
exit status 1
FAIL github.com/hashicorp/terraform/builtin/providers/statuscake 1.611s
make: *** [testacc] Error 1

Please let me know if you have any questions or concerns.

@ldjohnson
Copy link
Author

Hello All,

I wanted to check in on the status of this PR since I hadn't heard back in a while. Let me know if you have any questions or if anything is still needed for this. Thanks!

--Lee

@stack72
Copy link
Contributor

stack72 commented Dec 8, 2016

Hi @ldjohnson

There is an outstanding comment on your PR to the DreamItGetIt repo

Paul

@ldjohnson
Copy link
Author

@stack72 Thanks for the update! I have made the requested change to that PR. All that remains is for it to be reviewed and merged. Please let me know if you have any questions or concerns or if you need anything.

@stack72
Copy link
Contributor

stack72 commented Jan 17, 2017

Hi @ldjohnson

Thanks for the work here - I have manually merged this to get rid of the conflicts :)

% make testacc TEST=./builtin/providers/statuscake                                                                                    1 ↵ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/01/17 16:11:43 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/statuscake -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccStatusCake_basic
--- PASS: TestAccStatusCake_basic (1.81s)
=== RUN   TestAccStatusCake_withUpdate
--- PASS: TestAccStatusCake_withUpdate (2.94s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/statuscake	4.762s

@stack72 stack72 merged commit d203dcf into hashicorp:master Jan 17, 2017
@ghost
Copy link

ghost commented Apr 18, 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 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/statuscake waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants