Deeksha/add imports #5819
Deeksha/add imports #5819
Conversation
…(source registration operation), update registrations and delete registrations
|
Regression Results: 
|
|
Regression results: |
|
Regression result for resource ibm_backup_recovery_perform_action_on_protection_group_run_request 
|
ibm/conns/config.go
Outdated
|
|
||
| backupRecoveryClientOptions := &backuprecoveryv1.BackupRecoveryV1Options{ | ||
| Authenticator: authenticator, | ||
| URL: EnvFallBack([]string{"BACKUP_RECOVERY_ENDPOINT"}, backupRecoveryURL), |
There was a problem hiding this comment.
Append IBMCLOUD_ for this BACKUP_RECOVERY_ENDPOINT to keep in sync with all endpoints
ibm/conns/config.go
Outdated
| @@ -1592,6 +1603,30 @@ func (c *Config) ClientSession() (interface{}, error) { | |||
| } | |||
| } | |||
|
|
|||
| // Construct the service options. | |||
| backupRecoveryURL := "https://brs-stage-us-south-02.backup-recovery.test.cloud.ibm.com/v2" | |||
There was a problem hiding this comment.
why are we hardcoding stage endpoint.By default the provider shd target production endpoint
There was a problem hiding this comment.
backupRecoveryURL varies according to the the cluster created. So for end user (or different tenant), the backupRecoveryURL would be different. I will add a check to validate if it has been provided or not via environment variable or endpoints file.
| Required: true, | ||
| Description: "Specifies the key to be used to encrypt the source credential. If includeSourceCredentials is set to true this key must be specified.", | ||
| }, | ||
| "connector_ids": &schema.Schema{ |
There was a problem hiding this comment.
Can we provide connector_ids and connector_names, and connection_id all together or only one is supported at a time if so can you add conflicts_with , or some other options to control the behaviour
There was a problem hiding this comment.
We can provide the following combinations:
- connection_id and connector_names
- connector_ids and connector_names
But cannot provide : - connection_id and connector_ids
Will add conflictsWith for connection_id and connector_ids
| Optional: true, | ||
| Description: "Specifies the type of request from UI, which is used for services like magneto to determine the priority of requests.", | ||
| }, | ||
| "ids": &schema.Schema{ |
There was a problem hiding this comment.
same as above comment can we provide "ids" and "policy_names" together
There was a problem hiding this comment.
May be generic comment check for all datasources/resources
There was a problem hiding this comment.
Yes @hkantare
We can provide both ids and policy_names together. The API first filter out the policies based on ids and then proceed to match the names defined in policy_names. If a policy matching both the values is present then it is listed otherwise we get an empty policy list. In this case the API does not throw any error.
| return nil | ||
| } | ||
|
|
||
| func ResourceIbmBackupRecoveryValidator() *validate.ResourceValidator { |
There was a problem hiding this comment.
Are we using this validation?
There was a problem hiding this comment.
No @hkantare .I will remove it from the code.
| } | ||
|
|
||
| getRecoveryByIdOptions := &backuprecoveryv1.GetRecoveryByIdOptions{} | ||
| tenantId := d.Get("x_ibm_tenant_id").(string) |
There was a problem hiding this comment.
In Read we should avoid doing d.Get it effects the import flow
Already as part of d.SetID we store tenantID why can't we use that?
There was a problem hiding this comment.
Yes @hkantare , have extracted and used the tenant Id from d.Id() and used it to set the same variable tenantId = ParseId(d.Id(), "tenantId")
| // ForceNew: true, | ||
| Description: "Specifies the key to be used to encrypt the source credential. If includeSourceCredentials is set to true this key must be specified.", | ||
| }, | ||
| "registration_token": &schema.Schema{ |
There was a problem hiding this comment.
If this is secret value can we add sensitive : true so it mask the value in logs/output
There was a problem hiding this comment.
@hkantare a connection registration_token is something the user would like to use for registering connectors on this connection. So we would require this in output.
| page_title: "IBM : ibm_backup_recoveries" | ||
| description: |- | ||
| Get information about List of Recoveries. | ||
| subcategory: "IBM Backup Recovery API" |
There was a problem hiding this comment.
can we remove the word API from subcategory this names is use to group the res/ds in registry docs.
can we add this allowed-subcategoreis since its new service
| ``` | ||
| import { | ||
| to = <ibm_backup_recovery_resource> | ||
| id = "<tenantId>:<recovery_id>" |
There was a problem hiding this comment.
here we have single colon but in resources we are spiting by ("::")
document the separator correctly most of resource goes with ("/") or (":") any specific reason for ("::")
There was a problem hiding this comment.
yes @hkantare
The values of tenantId generally contains a '/' (like "jhxqx715r9/") and a lot resource Ids like recoveryId, policyId, groupIds etc. contains ":" (ex. a policy Id generally looks like this "5170815044477768:1732541085048:15126")
Since both '/' and ":" are present in tenantId and resourceId values, we have used "::"
There was a problem hiding this comment.
Also, I'll update id = "<tenantId>: to id = "<tenantId>:: in documentation
|
|
||
| import { | ||
| to = ibm_backup_recovery.backup_recovery_instance | ||
| id = "jhxqx715r9/::5170815044477768:1732541085048:484" |
There was a problem hiding this comment.
here I see both "/::" even / is separator or value of tenantID?
There was a problem hiding this comment.
It's the value of tenantID
Please enter the commit message for your changes. Lines starting
|
Regression result: |
IBM-Deeksha
force-pushed
the
deeksha/add-imports
branch
from
2cf51b4 to
9329264
Compare
datasources and resources for new backup and recovery service
Community Note
Relates OR Closes #0000
Output from acceptance testing: