INTMDB-28: Added Event Triggers Realm #476

coderGo93 · 2021-06-17T01:05:07Z

Description

Added resource, datasources and its tests for Event Triggers Realm
Modified config provider to get client realm or atlas
Modified all resources/datasources for atlas with one line which is the way to get atlas client, this is why appears many files changed

Link to any related issue(s):

Type of change:

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the Terraform contribution guidelines
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code

Further comments

Waiting for a PR of realm client to be merged then pull the changes so it can pass the normal tests

themantissa · 2021-06-17T15:33:41Z

@coderGo93 added apix to review. Reminder to share PRs in Slack as we discussed. Also tests failed.

coderGo93 · 2021-06-17T17:01:11Z

@themantissa thank you and yes because I'm a waiting a pr in realm client to be merged to master and then pull the changes in go modules and it will pass the test

themantissa · 2021-06-22T22:38:59Z

@tsedgwick though you'd want to see (and perhaps comment on/review) the PR for the work in Terraform.

threebee

Looking good, I have added a question for my own learning.

threebee · 2021-06-23T06:02:54Z

go.sum

+go.mongodb.org/realm v0.0.0-20210604190232-4d3dfc67ed71 h1:2jY3qOwzS8AQMRtuGxs8y/k1FRGWQHfvrrtEk0X+61Y=
+go.mongodb.org/realm v0.0.0-20210604190232-4d3dfc67ed71/go.mod h1:Tru1+aHka6+4uXYigPKoNySbNWgtkfsanVddwmcqvOM=
+go.mongodb.org/realm v0.0.0-20210618220639-e70c919266f2 h1:3ukcBKIOun3QRMZzdVRCU/7uVhjR7XQsdD/cCH5V8So=
+go.mongodb.org/realm v0.0.0-20210618220639-e70c919266f2/go.mod h1:Tru1+aHka6+4uXYigPKoNySbNWgtkfsanVddwmcqvOM=


[q] Why does go.sum mentions 2 versions of go.mongodb.org/realm? When only go.mongodb.org/realm v0.0.0-20210618220639-e70c919266f2 is present in go.mod?

That looks like go mod tidy might need to be run to remove the duplicates, unless the reference is coming in from another lib.

good eye @threebee , just pushed with vendor updated, thank you

themantissa

Overall lgtm but some small docs edits and test stuff.

mongodbatlas/data_source_mongodbatlas_event_triggers_test.go

mongodbatlas/resource_mongodbatlas_event_trigger.go

mongodbatlas/data_source_mongodbatlas_event_trigger_test.go

mongodbatlas/resource_mongodbatlas_event_trigger_test.go

website/docs/d/event_trigger.html.markdown

website/docs/r/event_trigger.html.markdown

themantissa

LGTM! Thank you for the edits.

threebee

LGTM if test passes. I personally found this PR kind of too big to review.

themantissa · 2021-06-25T15:20:18Z

@threebee do you think it would have helped to do the changes for the URI in one PR and the actual Trigger add in another? I agree the combined made for a large PR! fyi @coderGo93

coderGo93 · 2021-06-25T15:43:02Z

@threebee The tests for trigger are coded to be skipped because it requires to be available always for app realm since there's no a resource to create a realm app, also to load a sample dataset in cluster since it requires to link the database name and collection name, what do you think? @themantissa .

Yeah the PR is kinda big because of implementation of the second client instead of only atlas but I did that way because it seems safer to avoid any breaking since once it's merged, the other "old" PRs must be rebased to avoid an error that client type is wrong so in that way, if most of the tests of atlas passed it means it works the new implementation, obviously there's a chance that it will fail some testacc like before but it's expected since it hasn't fixed yet

threebee · 2021-06-25T16:05:16Z

@threebee The tests for trigger are coded to be skipped because it requires to be available always for app realm since there's no a resource to create a realm app, also to load a sample dataset in cluster since it requires to link the database name and collection name, what do you think? @themantissa .

Yeah the PR is kinda big because of implementation of the second client instead of only atlas but I did that way because it seems safer to avoid any breaking since once it's merged, the other "old" PRs must be rebased to avoid an error that client type is wrong so in that way, if most of the tests of atlas passed it means it works the new implementation, obviously there's a chance that it will fail some testacc like before but it's expected since it hasn't fixed yet

@coderGo93 Thank you for explanation. Feel free to merge, my comment is non blocking.

thetonymaster

LGTM just check for the changes you need to do before merging

…nfig match and project

coderGo93 requested a review from abner-dou June 17, 2021 01:05

coderGo93 self-assigned this Jun 17, 2021

coderGo93 requested review from andreaangiolillo and themantissa as code owners June 17, 2021 01:05

coderGo93 removed request for themantissa and andreaangiolillo June 17, 2021 01:05

coderGo93 force-pushed the INTMDB-28 branch from 1569f8d to f2721e0 Compare June 17, 2021 01:11

themantissa requested review from a team and threebee and removed request for a team June 17, 2021 15:32

coderGo93 changed the title ~~WIP: INTMDB-28: Added Event Triggers Realm~~ INTMDB-28: Added Event Triggers Realm Jun 18, 2021

coderGo93 requested a review from themantissa June 18, 2021 22:17

themantissa requested a review from tsedgwick June 22, 2021 22:38

threebee reviewed Jun 23, 2021

View reviewed changes

coderGo93 force-pushed the INTMDB-28 branch from 83d50d3 to a5361a6 Compare June 23, 2021 23:08

themantissa requested changes Jun 23, 2021

View reviewed changes

coderGo93 requested a review from themantissa June 24, 2021 17:56

themantissa approved these changes Jun 24, 2021

View reviewed changes

threebee approved these changes Jun 25, 2021

View reviewed changes

coderGo93 requested a review from thetonymaster June 28, 2021 18:15

thetonymaster approved these changes Jun 29, 2021

View reviewed changes

Edgar López added 3 commits June 29, 2021 15:52

added realm client in config provider

f6306cb

feat:added resource/datasource(s) and its tesst for event trigger

d1393f7

docs: added docs for event trigger

6324269

Edgar López added 7 commits June 29, 2021 15:52

refactor

f80dfe6

added client for realm and the way to assign

ae06040

added skip test

a86ac60

refactor: the way to connect for atlas client

a0166a8

fixes linter

527eb1e

added supressdiff func so it can pass it two json are the same for co…

2966ccf

…nfig match and project

refactor: refactor some

71d253a

coderGo93 force-pushed the INTMDB-28 branch from 2e0aeca to 71d253a Compare June 29, 2021 21:56

coderGo93 added the run-testacc To run acceptance tests label Jun 29, 2021

updated getting client for datalakes

3f89222

coderGo93 added run-testacc To run acceptance tests and removed run-testacc To run acceptance tests labels Jun 30, 2021

coderGo93 merged commit 8ffed1c into master Jun 30, 2021

coderGo93 deleted the INTMDB-28 branch June 30, 2021 03:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

INTMDB-28: Added Event Triggers Realm #476

INTMDB-28: Added Event Triggers Realm #476

coderGo93 commented Jun 17, 2021 •

edited

Loading

themantissa commented Jun 17, 2021

coderGo93 commented Jun 17, 2021

themantissa commented Jun 22, 2021

threebee left a comment

threebee Jun 23, 2021

colm-quinn Jun 23, 2021

coderGo93 Jun 23, 2021

themantissa left a comment

themantissa left a comment

threebee left a comment

themantissa commented Jun 25, 2021

coderGo93 commented Jun 25, 2021

threebee commented Jun 25, 2021

thetonymaster left a comment

INTMDB-28: Added Event Triggers Realm #476

INTMDB-28: Added Event Triggers Realm #476

Conversation

coderGo93 commented Jun 17, 2021 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

themantissa commented Jun 17, 2021

coderGo93 commented Jun 17, 2021

themantissa commented Jun 22, 2021

threebee left a comment

Choose a reason for hiding this comment

threebee Jun 23, 2021

Choose a reason for hiding this comment

colm-quinn Jun 23, 2021

Choose a reason for hiding this comment

coderGo93 Jun 23, 2021

Choose a reason for hiding this comment

themantissa left a comment

Choose a reason for hiding this comment

themantissa left a comment

Choose a reason for hiding this comment

threebee left a comment

Choose a reason for hiding this comment

themantissa commented Jun 25, 2021

coderGo93 commented Jun 25, 2021

threebee commented Jun 25, 2021

thetonymaster left a comment

Choose a reason for hiding this comment

coderGo93 commented Jun 17, 2021 •

edited

Loading