Skip to content
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 parseduration() function #27048

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

remilapeyre
Copy link
Contributor

Closes #16474

@remilapeyre
Copy link
Contributor Author

I found myself needing this to better express durations where an integer is expected. I made the choice to truncate the result when it cannot be exactly represented in the given unit because it seems to me that most of the time providers are expecting an integer rather than an Int more often than a Float to express durations.

@codecov
Copy link

codecov bot commented Nov 27, 2020

Codecov Report

Merging #27048 (564e86a) into master (111825d) will decrease coverage by 0.04%.
The diff coverage is 3.03%.

Impacted Files Coverage Δ
lang/funcs/datetime.go 28.88% <0.00%> (-71.12%) ⬇️
lang/functions.go 92.19% <100.00%> (+0.05%) ⬆️
terraform/eval_diff.go 66.51% <0.00%> (-0.92%) ⬇️
terraform/node_resource_apply_instance.go 75.00% <0.00%> (-0.79%) ⬇️
terraform/evaluate.go 52.89% <0.00%> (-0.42%) ⬇️
states/statefile/version4.go 58.19% <0.00%> (+0.23%) ⬆️

@reist01
Copy link

reist01 commented Nov 24, 2021

Is there anything needed here apart from resolving conflicts?

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@crw crw added the functions label Mar 14, 2022
@zx8
Copy link

zx8 commented Jun 24, 2022

Can we get some movement on this?

I have found myself needing this several times in the years since this PR was originally raised and it seems like a relatively simple thing to review & merge (once conflicts have been resolved).

@crw
Copy link
Collaborator

crw commented Jun 24, 2022

Thanks for this submission and the PR bump. As for getting a PR accepted, please see this comment: #28855 (comment), specifically:

We're being pretty cautious about adding new functions that overlap with existing use-cases because Terraform has grown quite an unwieldy collection of builtins over the years, and so we've been considering various ways to allow for externally-defined functions (e.g. #2771) to avoid continuing to grow that set. However, that's not something we'll be working on in the very near future, due to our current focus being elsewhere, so we'll revisit this at a later time and post any updates in those other issues.

Reviewing the comments on the original issue, my guess is that this would more likely be considered as a plugin function, rather than as a built-in function. However, I'll bring this up for review in triage to see where it stands. Thanks again!

@crw
Copy link
Collaborator

crw commented Jul 8, 2022

Reviewing the comments on the original issue, my guess is that this would more likely be considered as a plugin function, rather than as a built-in function.

Confirmed that this is currently considered more appropriate as a plugin function rather than as a built-in. I will update this PR when such functionality is available. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a native data type for duration that providers can understand
6 participants