Update GCP integration - Githubissues

zorkian / go-datadog-api

A Go implementation of the Datadog API.

BSD 3-Clause "New" or "Revised" License

183 stars 156 forks source link

Update GCP integration #285

Closed mzneos closed 4 years ago

mzneos commented 4 years ago

What does this PR do?

Adds the field AutoMute to the IntegrationGCPCreateRequest object.
Updates the IntegrationGCP object
Refactors the UpdateIntegrationGCP function as it was not working anymore (the endpoints does not seem to work)
- for backward compatibility, the IntegrationGCPUpdateRequest object was just updated instead of using the already existing IntegrationGCPCreateRequest object, which is identical
Updates the integration/integration_test.go tests functions for the GCP integration, adding checks for the AutoMute field

bkabrda commented 4 years ago

Hey, thanks for sending this PR. The code looks good, but I'd like to ask you to extend the test TestIntegrationGCPUpdate in integration/integration_test.go. It'd be awesome if that tested most (or ideally all) of the structure fields of IntegrationGCPUpdateRequest.

mzneos commented 4 years ago

Hey, thanks for sending this PR. The code looks good, but I'd like to ask you to extend the test TestIntegrationGCPUpdate in integration/integration_test.go. It'd be awesome if that tested most (or ideally all) of the structure fields of IntegrationGCPUpdateRequest.

Yes of course, I can do that. I'll look into it as soon as I have some time.

mzneos commented 4 years ago

I think the tests should be ok now. I also updated the test for TestIntegrationGCPCreateAndDelete to check the new AutoMute field added. All the fields returned by ListIntegrationGCP are tested now, in both functions

bkabrda commented 4 years ago

I'm getting the following failure:

=== RUN   TestIntegrationGCPUpdate
--- FAIL: TestIntegrationGCPUpdate (4.80s)
    integrations_test.go:431: Retrieving a GCP integration failed when it shouldn't: json: cannot unmarshal bool into Go struct field IntegrationGCP.automute of type string

It seems that you provided type *string for IntegrationGCP.AutoMute, but it should be *bool.

mzneos commented 4 years ago

Indeed there was a typo in the IntegrationGCP struct. Thank you for spotting that! The tests should succeed now

bkabrda commented 4 years ago

Perfect, LGTM now => merging. Thank you very much for this contribution!