Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add resource and data source for MAAS zones #258
base: master
Are you sure you want to change the base?
feat: add resource and data source for MAAS zones #258
Changes from 1 commit
23a0d92
1761603
b283b11
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about setting as terraform ID the database ID of the zone instead of the name? In addition, line 41
"id": fmt.Sprintf("%v", zone.ID),
is not neededThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to find if zone already exists in MAAS here costs an extra API call to MAAS. It is accepted to blindly try create the zone as the intention is that Terraform is managing it. If it already exists, MAAS will throw an error and this will be propagated to gomaasclient, then from the client to terraform provider and finally being presented to user as part of diagnostic errors. Then the user can import the zone to Terraform and continue its management with Terraform.
Let's simplify the function. Resource pool is again a good example as the implementation is more or less identical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As said earlier, let's save the database ID as a Terraform resource ID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed since we want to save the database ID. Then
id
should be given toZone.Update
instead ofd.Id()
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified to
given that id is calculated like the previous suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also needed if we are to save the database ID as the Terraform resource ID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.