-
Notifications
You must be signed in to change notification settings - Fork 162
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
add resource dms_kafka_instance #1162
Conversation
6fc0ad6
to
1f09fcf
Compare
docs/resources/dms_kafka_instance.md
Outdated
|
||
* `product_id` - (Required, String, ForceNew) Specifies a product ID. Changing this creates a new instance resource. | ||
|
||
* `kafka_manager_user` - (Required, String, ForceNew) Specifies the username for logging in to the Kafka Manager. |
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.
suggest to rename it to manager_user
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.
fixed
docs/resources/dms_kafka_instance.md
Outdated
- `time_base`: Automatically delete the earliest messages. | ||
- `produce_reject`: Stop producing new messages. | ||
|
||
* `connector_enable` - (Optional, Bool, ForceNew) Specifies whether to enable dumping. |
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.
suggest rename it to dumping
to keep with the console
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.
fixed
docs/resources/dms_kafka_instance.md
Outdated
- `true`: enable | ||
- `false`: disable |
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.
we can set the value to true
of false
for a bool type, so the two lines are redundant.
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.
fixed
docs/resources/dms_kafka_instance.md
Outdated
|
||
Changing this creates a new instance resource. | ||
|
||
* `partition_num` - (Required, Int, ForceNew) Specifies the maximum number of topics in the DMS kafka instance. |
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.
if the value depends on specification
, we can use it as an internal parameter or set it as an attribute
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.
fixed
Computed: true, | ||
ForceNew: true, | ||
}, | ||
"storage_spec_code": { |
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.
put the required field in the head of the schema
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.
fixed
docs/resources/dms_kafka_instance.md
Outdated
* `user_id` - Indicates a user ID. | ||
* `user_name` - Indicates a username. |
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.
tenant user id or the kafka user 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.
fixed
specification := d.Get("specification").(string) | ||
publicIpIDsRaw := d.Get("public_ip_ids").([]interface{}) | ||
|
||
if specification == "100MB" && len(publicIpIDsRaw) != 3 { |
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.
using a map will be more clearer
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.
fixed
1f09fcf
to
bf36ed8
Compare
bf36ed8
to
17944c8
Compare
latest test result:
|
What this PR does / why we need it:
add resource dms_kafka_instance
Which issue this PR fixes:
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged)related #1052
Special notes for your reviewer:
Release note:
PR Checklist
Acceptance Steps Performed