Skip to content

Conversation

@skirsten
Copy link
Contributor

@skirsten skirsten commented Feb 9, 2023

Reproduce

"timeouts": timeouts.AttributesAll(ctx),
timeouts = {
  create = "60s"
}

on next delete/read/update:

Error: Timeout Cannot Be Parsed
  timeout for "read" cannot be parsed, time: invalid duration ""

@skirsten skirsten requested a review from a team as a code owner February 9, 2023 18:20
Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @skirsten 👋

Thank you for your contribution and sorry you ran into trouble here.

The changes look good.

Could I ask you to add test coverage for each of the Create, Read, Update and Delete functions on resource timeouts and for Read on datasource timeouts?

For example:

		"read-null": {
			timeoutsValue: timeouts.Value{
				Object: types.ObjectValueMust(
					map[string]attr.Type{
						"read": types.StringType,
					},
					map[string]attr.Value{
						"read": types.StringNull(),
					},
				),
			},
			expectedTimeout: 20 * time.Minute,
		},
		"read-unknown": {
			timeoutsValue: timeouts.Value{
				Object: types.ObjectValueMust(
					map[string]attr.Type{
						"read": types.StringType,
					},
					map[string]attr.Value{
						"read": types.StringUnknown(),
					},
				),
			},
			expectedTimeout: 20 * time.Minute,
		},

Could you also add 2 changelog entries using Changie which we are now using for changelog and version management.

Once Changie is installed can you run the following:

changie new
? Kind: BUG FIXES
Body: datasource/timeouts: Use default for null and unknown ([#35](https://github.com/hashicorp/terraform-plugin-framework-timeouts/pull/35)).
Issue/PR Number: 35
changie new
? Kind: BUG FIXES
Body: resource/timeouts: Use default for null and unknown ([#35](https://github.com/hashicorp/terraform-plugin-framework-timeouts/pull/35)).
Issue/PR Number: 35

@bendbennett bendbennett added the bug Something isn't working label Feb 10, 2023
@skirsten
Copy link
Contributor Author

Thanks for the review.
I added the tests and updated the changelog. Can you please take a look again?

Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@bendbennett bendbennett merged commit daf4bbb into hashicorp:main Feb 13, 2023
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants