search

zclconf / go-cty

A type system for dynamic values in Go applications
MIT License
348 stars 71 forks source link

Extend FormatDateFunc to support seconds since Unix Epoch #130

Closed benjamb closed 2 years ago

benjamb commented 2 years ago

It would be handy to be able to convert a provided date to a Unix timestamp. I'm using packer which wraps this function via its timestamp() function.

apparentlymart commented 2 years ago

Hi @benjamb! Thanks for this suggestion.

I intended the formatdate function to be primarily for producing the various machine-readable-but-also-human-readable date formats defined in different IETF RFCs and other specifications.

While I can certainly see that having access to a "unix timestamp" might be useful in some situations, to me that feels like a separate function from formatdate because it's producing a single hard-coded representation of an entire timestamp, which would be weird to combine with the other formatted parts that formatdate works with. Also, the natural return type of this operation would be a number rather than a string.

Given that, I'd suggest creating a new function to convert a per-conventions timestamp (RFC 3339) into a Unix timestamp, rather than adding it to FormatDateFunc.

I'm currently being quite reserved about adding new functions directly in the stdlib package of this repository, because the scope there has already become much larger than I had originally imagined thanks to HashiCorp contributing upstream various functions that were originally written for HashiCorp Terraform. Therefore I don't think I'd want to add such a function in this repository right now, but HashiCorp has its own library of cty functions in addition to the ones maintained in here, and so perhaps they'd be willing to accept a unix timestamp conversion function into that library instead, which HashiCorp Packer could then include.

Thanks again!

benjamb commented 2 years ago

@apparentlymart Thanks for the thorough response!

I agree it doesn't actually make much sense to extend formatdate to support this, and that perhaps I should open an issue downstream instead.

With regards to the HashiCorp cty library, that very much looks unmaintained, so that route is likely a non-starter.

apparentlymart commented 2 years ago

Hi @benjamb,

The library I linked to was created by the Packer team specifically to hold the functions that Packer uses that I declined for upstreaming in this library (due to them having some design oddities inherited from Terraform), so I expect inactivity there is more a consequence of the library currently being "done" as far as Packer is concerned, rather than because it's closed to future enhancements.

Although I do work at HashiCorp along with being the maintainer of this personal project, I don't work on Packer and so I can't speak to what the Packer team would want to include or how they would want to include it, but I would think a discussion in their main repository would probably the best place to start, and then you can hear directly from them about whether they'd be interested in such a new function and, if so, whether they'd rather have it live in the Packer codebase or in the shared functions library I linked to before.

benjamb commented 2 years ago

@apparentlymart Ah, that makes sense.

Yeah I've opened an issue in the packer repository for now, we'll see what happens.