Fix addr and ID usage for import #24
Conversation
| @@ -75,8 +67,8 @@ func (opt *VarFileOption) configureImport(conf *importConfig) { | |||
| conf.varFile = opt.path | |||
| } | |||
|
|
|||
| func (t *Terraform) Import(ctx context.Context, opts ...ImportOption) error { | |||
| importCmd := t.ImportCmd(ctx, opts...) | |||
| func (t *Terraform) Import(ctx context.Context, address, id string, opts ...ImportOption) error { | |||
There was a problem hiding this comment.
I have these as explicit strings here instead of the options model since they are required for this command.
There was a problem hiding this comment.
This is reasonable, but it means a breaking change for tfexec if terraform import gains or loses required arguments during future terraform versions.
To my knowledge there is no other terraform subcommand with required arguments, but I may be wrong...
|
Need to add a runtime test |
|
Failing test: I think this test is flaky but this is the first time I've caught it in a build. Will fix separately. |
Also reorg test fixtures to prep for more fixtures
Add runtime testing of import across multiple versions of TF
| @@ -75,8 +67,8 @@ func (opt *VarFileOption) configureImport(conf *importConfig) { | |||
| conf.varFile = opt.path | |||
| } | |||
|
|
|||
| func (t *Terraform) Import(ctx context.Context, opts ...ImportOption) error { | |||
| importCmd := t.ImportCmd(ctx, opts...) | |||
| func (t *Terraform) Import(ctx context.Context, address, id string, opts ...ImportOption) error { | |||
There was a problem hiding this comment.
This is reasonable, but it means a breaking change for tfexec if terraform import gains or loses required arguments during future terraform versions.
To my knowledge there is no other terraform subcommand with required arguments, but I may be wrong...
| @@ -40,6 +32,11 @@ func Backup(path string) *BackupOption { | |||
| return &BackupOption{path} | |||
| } | |||
|
|
|||
| // DisableBackup is a convenience method for Backup("-"), indicating backup state should be disabled. | |||
| func DisableBackup() *BackupOption { | |||
There was a problem hiding this comment.
Happy with this direction - we can provide more convenience options as use cases become clear.
Also, fix minor issue of orphaned installed directories.
This is dependent upon #23