Refactor/migrate region #230

YanniHu1996 · 2023-05-15T10:52:05Z

No description provided.

serdardalgic

Thanks for your work, @YanniHu1996. The PR is going in a good direction, IMHO.
I've requested some changes.
After you fix them, please check the following:

Please make sure to run a make docs for the latest documentation.
Please ensure that the examples are correct. In the examples, we can add bah:aws to the cloud_provider variables, for example the biganimal_regions data source.
please test the biganimal_region and biganimal_regions data sources, working as expected. (biganimal_region giving a deprecation notice).
please test the biganimal_region resource, preferably with the acceptance tests. If you need to introduce new tests for the biganimal_region resource other than the Basic test, we can include it within this PR.

Thanks a lot!

docs/data-sources/pgd.md

docs/data-sources/region.md

pkg/provider/data_source_regions.go

pkg/provider/data_source_regions_test.go

pkg/provider/resource_region.go

templates/data-sources/regions.md.tmpl

pkg/provider/validators.go

serdardalgic

Some style related changes are requested. Thank you 🙏

.env.example

examples/data-sources/biganimal_region/data-source.tf

pkg/provider/data_source_regions.go

pkg/provider/resource_region.go

pkg/provider/utils.go

serdardalgic · 2023-06-28T22:58:38Z

pkg/provider/data_source_regions.go

+	} else {
+		respRegions, err := r.client.List(ctx, *cfg.ProjectId, *cfg.CloudProvider, cfg.Query.ValueString())
+		if err != nil {
+			return


There is something wrong in this part of the code.
We call the r.client.List and read the response or check if there is any error.
If there is any error, we're not adding it to resp.Diagnostics.
If I'm not mistaken, we need to res.Diagnostics.AddError this error, and then return the method.

Otherwise, when I have an BA API error, such as no token etc., current implementation misses that.

serdardalgic · 2023-06-28T23:12:16Z

pkg/provider/utils.go

@@ -35,3 +38,12 @@ func unsupportedWarning(message string) diag.Diagnostics {
 		},
 	}
 }
+
+func fromErr(err error, summary string, args ...any) diag2.Diagnostics {


This function is quite dangerous.
When we use fromErr, it will remove all the other Diagnostics that were created during any Method (can be warnings etc.) and will return a clean New Error Diagnostic that returns a given error.

Please use any Diagnostics method to add the error (AddError ideally) instead of completely overriding the errors.

Hi Serdar, could you please provide more information about the statement "This function is quite dangerous"? As I understand it, this function returns "Diagnostics," which will be appended to resp.Diagnostics using the Append method. It should not overwrite any existing diagnostics. The purpose of fromErr is to convert errors into Diagnostics by accepting an error type parameter. On the other hand, Diagnostics.AddError does not accept an error parameter, which means you need to convert the error to a string in your subsequent code

Co-authored-by: Serdar Dalgıç <[email protected]>

… apply

serdardalgic

The PR looks good to me. Thanks for your work, @YanniHu1996 . 🎉

YanniHu1996 changed the base branch from main to refactor/migrate-project May 15, 2023 11:33

YanniHu1996 force-pushed the refactor/migrate-region branch from 4eae769 to 0a16063 Compare May 15, 2023 11:58

YanniHu1996 force-pushed the refactor/migrate-region branch from 0a16063 to 1bd9e0d Compare May 24, 2023 08:06

YanniHu1996 force-pushed the refactor/migrate-region branch from 1bd9e0d to 4a37c93 Compare May 31, 2023 10:24

YanniHu1996 force-pushed the refactor/migrate-project branch 4 times, most recently from d671726 to a9f419d Compare June 6, 2023 06:32

YanniHu1996 added 5 commits June 8, 2023 16:18

refactor: rewrite datasource_projects with terraform-plugin-framework

0c47437

test: Added acctest for project resource

fa4e2a2

fix: Use other test helper

f8b442e

fix: Fixed acc failure

cd523e6

fix: Fixed pre-commit

3af6be4

YanniHu1996 force-pushed the refactor/migrate-project branch from a9f419d to 27850ba Compare June 8, 2023 08:18

YanniHu1996 force-pushed the refactor/migrate-region branch from 4a37c93 to e10ed27 Compare June 8, 2023 08:21

fix: Fixed compile error

228bfef

YanniHu1996 force-pushed the refactor/migrate-region branch from 9e0a157 to 4072841 Compare June 8, 2023 09:42

YanniHu1996 force-pushed the refactor/migrate-project branch from 27850ba to 228bfef Compare June 8, 2023 09:42

YanniHu1996 added 12 commits June 9, 2023 14:43

fix: Fixed go fmt and goimports

f004766

fix: Added UT for configure function

fb385a0

feat: Added import for project resource

95f8ff0

refactor: rewrite datasource_projects with terraform-plugin-framework

90d6b50

test: Added acctest for project resource

5da140c

fix: Use other test helper

540dabf

fix: Fixed acc failure

5698727

fix: fixed typo

ad5cba2

fix: Fixed acc failure

833e0b6

refactor: rewrite resource_region with terraform-plugin-framework

b2f6927

refactor: rewrite datasource_region with terraform-plugin-framework

82eff4e

fix: Fixed golint

fad9a2f

serdardalgic suggested changes Jun 27, 2023

View reviewed changes

YanniHu1996 added 3 commits June 28, 2023 17:23

fix: Fixed comments

ebe54d3

Merge remote-tracking branch 'origin/main' into refactor/migrate-region

7c6f73a

fix: Fixed make docs

729e235

serdardalgic suggested changes Jun 28, 2023

View reviewed changes

serdardalgic added 2 commits June 28, 2023 17:00

docs: Add docs for regions data source.

7e7674f

feat: Add import for biganimal_region resource.

c543b10

serdardalgic reviewed Jun 28, 2023

View reviewed changes

YanniHu1996 and others added 7 commits June 29, 2023 14:49

fix: Added equal sign after variable name

e579431

Co-authored-by: Serdar Dalgıç <[email protected]>

fix: Updated message.

5e35be3

Co-authored-by: Serdar Dalgıç <[email protected]>

fix: Updated message.

6afac0b

Co-authored-by: Serdar Dalgıç <[email protected]>

fix: Updated message.

7201e6f

Co-authored-by: Serdar Dalgıç <[email protected]>

fix: Use frameworkdiag for more specific

e98badf

Co-authored-by: Serdar Dalgıç <[email protected]>

fix: Use sdkdiag for more specific

21dc00f

Co-authored-by: Serdar Dalgıç <[email protected]>

fix: Run make docs

9b76e3f

YanniHu1996 force-pushed the refactor/migrate-region branch from 433cfee to 9b76e3f Compare June 29, 2023 08:08

YanniHu1996 and others added 7 commits June 29, 2023 17:15

fix: Fixed unexpected of terraform plan and inconsistent result after…

b1bf8e0

… apply

fix: Fixed test failure

6b34350

feat: Add helper function for BA API Errors

450cfc6

fix: Handle BigAnimal Error

bd8b5d1

fix: Removed data_source_regions

c6c9b0f

fix: Correct regions -> region in the ACC test

2f9857d

fix: Removed data_source_regions

f663691

YanniHu1996 force-pushed the refactor/migrate-region branch from 2d8bca0 to f663691 Compare July 11, 2023 06:08

serdardalgic and others added 2 commits July 11, 2023 11:53

fix: remove the unused biganimal_regions data sources.

3d65af2

fix: Fixed panic

9acd973

serdardalgic approved these changes Jul 11, 2023

View reviewed changes

serdardalgic merged commit 92afa7f into main Jul 11, 2023

serdardalgic deleted the refactor/migrate-region branch July 11, 2023 12:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/migrate region #230

Refactor/migrate region #230

YanniHu1996 commented May 15, 2023

serdardalgic left a comment

serdardalgic left a comment

serdardalgic Jun 28, 2023

serdardalgic Jun 28, 2023

YanniHu1996 Jun 29, 2023

serdardalgic left a comment

Refactor/migrate region #230

Refactor/migrate region #230

Conversation

YanniHu1996 commented May 15, 2023

serdardalgic left a comment

Choose a reason for hiding this comment

serdardalgic left a comment

Choose a reason for hiding this comment

serdardalgic Jun 28, 2023

Choose a reason for hiding this comment

serdardalgic Jun 28, 2023

Choose a reason for hiding this comment

YanniHu1996 Jun 29, 2023

Choose a reason for hiding this comment

serdardalgic left a comment

Choose a reason for hiding this comment