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

Extend FormatDateFunc to support seconds since Unix Epoch #130

Closed
benjamb opened this issue Aug 15, 2022 · 4 comments
Closed

Extend FormatDateFunc to support seconds since Unix Epoch #130

benjamb opened this issue Aug 15, 2022 · 4 comments

Comments

@benjamb
Copy link

benjamb commented Aug 15, 2022

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
Copy link
Collaborator

apparentlymart commented Aug 15, 2022

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
Copy link
Author

benjamb commented Aug 16, 2022

@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
Copy link
Collaborator

apparentlymart commented Aug 16, 2022

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
Copy link
Author

benjamb commented Aug 17, 2022

@apparentlymart Ah, that makes sense.

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

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

No branches or pull requests

3 participants
@apparentlymart @benjamb and others