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

proposal: time: direct support for Unix millis and micros - RE: #18935 #27782

Closed
jeffreydwalter opened this issue Sep 20, 2018 · 21 comments
Closed

proposal: time: direct support for Unix millis and micros - RE: #18935 #27782

jeffreydwalter opened this issue Sep 20, 2018 · 21 comments

Comments

@jeffreydwalter
Copy link

@jeffreydwalter jeffreydwalter commented Sep 20, 2018

I am writing a Go library to interact with the Netgear Arlo camera API, and it uses UNIX Epoch in millisecond (13 digit) form extensively.

Why not add support for these time formats? They exist in other languages and therefore other APIs which may potentially be consumed by Go.

I vote for the original proposal in #18935:

Initial tl;dr: Go could make timestamp handling much easier by providing the following functions in its time library:

func (t Time) UnixMicro() int64 { ... }
func (t Time) UnixMilli() int64 { ... }
func FromUnixMicro(us int64) Time { ... }
func FromUnixMilli(ms int64) Time { ... }
@gopherbot gopherbot added this to the Proposal milestone Sep 20, 2018
@gopherbot gopherbot added the Proposal label Sep 20, 2018
@dsnet
Copy link
Member

@dsnet dsnet commented Sep 20, 2018

#18935 was closed a year ago with the decision that this need does not seem common enough to warrant adding more API for it. You mention your specific use-case for it, but that doesn't necessarily mean that it is universally common enough to justify new API.

If you want this to be reconsidered, you'll need to find examples on open source Go code that demonstrate that dealing with timestamps in milliseconds and microseconds is sufficiently common.

@jeffreydwalter
Copy link
Author

@jeffreydwalter jeffreydwalter commented Sep 20, 2018

I mention my use case as support for the fact that there are public APIs that Go programmers will consume that use the format. The fact that Go doesn't support the format makes it a pain to use the API.

@dsnet
Copy link
Member

@dsnet dsnet commented Sep 20, 2018

I acknowledge the validity your use case, but the standard library is intended for everybody. Thus, the bar for adding API to it must be something that is going to be useful for a wide range of developers. The consumers of the proposed APIs have not been demonstrated.

@neild
Copy link
Contributor

@neild neild commented Sep 21, 2018

We've got an internal unixtime package that contains conversion functions such as func FromFloatSeconds(wt float64) time.Time. It's got a startlingly large number of users.

I don't know if that's an argument for (definitely proven to be useful here), or against (trivial implementation, doesn't need to be in the stdlib).

@jeffreydwalter
Copy link
Author

@jeffreydwalter jeffreydwalter commented Sep 21, 2018

@jeffreydwalter
Copy link
Author

@jeffreydwalter jeffreydwalter commented Sep 21, 2018

@neild, it's definitely trivial to roll your own library for this, but how many people have wasted time reinventing that wheel? How many more will have to? It's insanity... 😵

@agnivade
Copy link
Contributor

@agnivade agnivade commented Sep 21, 2018

I'll chime in with my real-world use case. I am writing some code right now which gets payment subscription info from an app. The startTime and expiryTime fields of the subscription returned is in unix millis. https://developers.google.com/android-publisher/api-ref/purchases/subscriptions#resource

@jeffreydwalter
Copy link
Author

@jeffreydwalter jeffreydwalter commented Sep 22, 2018

I'm just going to put this stackoverflow about dealing with UNIX epochs in APIs (note this is a Java API) here as more evidence... https://stackoverflow.com/a/31745264/6138350

@dsnet can you really tell me with a straight-faced emoji that this is a good solution in light of having support for this in the standard library? :|

@dsnet
Copy link
Member

@dsnet dsnet commented Sep 22, 2018

@dsnet can you really tell me with a straight-faced emoji that this is a good solution in light of having support for this in the standard library? :|

@jeffreydwalter. I'm not the one who makes API decisions for the time package. I'm trying to help you by letting you know what needs to be demonstrated if you really believe this needs to be in the time package. This proposal is unique in the sense that it was proposed and explicitly rejected because the use-case for it was not deemed sufficient. Thus, if you're aiming to re-propose the same issue, you'll need to demonstrate that the reason for rejecting it the first time around was wrong and that there really would be a lot of usage of the proposed functions. There is no need to address this question directly at me as if I'm trying to make your life difficult. Let's be respectful here and not assume anything about each other's intentions.

That being said, I'm not sure how the link you pointed to is significantly helped by this proposal. The issue seems to be about wanting to serialize JSON as milliseconds since Unix epoch. However, a Go type can only have one MarshalJSON and UnmarshalJSON method, and the current methods on time.Time are defined to use RFC3339, which is an entirely reasonable default. In order to serialize milliseconds, you would still need to: 1) define your own type, 2) define your own UnmarshalJSON method, 3) manual parse the the string input, 4) convert the milliseconds to a time.Time. Only the 4th task is helped by this proposal, but it's also the simplest of the 4 (being a single line). Thus, accepting this proposal does not make a large difference on what someone needs to do to address the example you brought up.

@ulikunitz
Copy link
Contributor

@ulikunitz ulikunitz commented Sep 23, 2018

The requested functions are trivial to program and don't justify the addition to the standard library.

func UnixMicro(t time.Time) int64 {
	ns := t.UnixNano()
	if ns < 0 {
		return (ns - 999) / 1000
	}
	return ns / 1000
}

func UnixMilli(t time.Time) int64 {
	ns := t.UnixNano()
	if ns < 0 {
		return (ns - 999999) / 1000000
	}
	return ns / 1000000
}

func FromUnixMicro(µs int64) time.Time { return time.Unix(0, 1000*µs) }

func FromUnixMilli(ms int64) time.Time { return time.Unix(0, 1000000*ms) }
@jeffreydwalter
Copy link
Author

@jeffreydwalter jeffreydwalter commented Sep 23, 2018

@ulikunitz where does it say that "trivial" is the bar for inclusion in the standard library? The fact that they are trivial doesn't change the fact that people are clearly having to reinvent them over and over, which is exactly what should make them a good candidate for inclusion. These time formats are common enough that they cause consternation at the fact that something so trivial, but useful, isn't part of the time package. Thus the multiple feature requests and repeated questions on various forums about them...

@ulikunitz
Copy link
Contributor

@ulikunitz ulikunitz commented Sep 24, 2018

Any type, function, constant or variable added to the standard library must be maintained perpetually with the exact same semantics. So there is a high cost to add them to the standard library. The proposed function don't add any new orthogonal functionality. The standard library handles the nanosecond case and milliseconds and microseconds conversion can be achieved by multiplication and division, which everybody has learned about in school. The standard library even contains the factors as constants of type duration, which makes them a little bit inconvenient to use here.

While I could find a question on StackOverflow regarding this, I wonder why I can't find a package with those functions, if there is a really such a huge need for it.

If we include UnixMilli and UnixMicro, why don't we include UnixMinute, UnixHour, UnixDay, UnixWeek, UnixYear, etc. including the back conversions. Where do you want to stop?

@jeffreydwalter
Copy link
Author

@jeffreydwalter jeffreydwalter commented Sep 24, 2018

I don't think the semantics for time are likely to change any time soon, so I don't really buy the argument that the cost is in maintaining them.

I think you stop where the need stops. No one has a need for, nor is asking for, those other functions you mention. Javascript, for instance, uses the Unix Epoch in milliseconds natively. Because of that, there are plenty of APIs out there which serialize timestamps in their data formats. Working with those APIs is not as delightful as it should be in Go.

Sure, I can write my own functions to deal with those formats. That's what I've done... It's also what everyone who needs these basic conversions does. (Those functions you pasted were probably from your local time library, right?)
There have been several people between this issue and #18935 who have admitted to writing their own library to do this stuff. None of them bothered to make a package out of it, but that doesn't mean that no one needs this functionality.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 24, 2018

@neild's comment above (#27782 (comment)) demonstrates that these can be in a third party library. Write that third party library and make it go-gettable. If many people start using it, that is clear evidence that these functions should be in the standard library.

@ulikunitz
Copy link
Contributor

@ulikunitz ulikunitz commented Sep 26, 2018

I've created github.com/ulikunitz/unixtime to measure the demand for the functionality. Note that I didn't have an existing package. I have improved the implementation to cover all possible int64 values.

See https://godoc.org/github.com/ulikunitz/unixtime for the documentation.

@rsc
Copy link
Contributor

@rsc rsc commented Sep 27, 2018

Duplicate of #18935. As already noted, if a third-party package becomes very widely used then we'll consider lifting them into package time. But I haven't seen any new evidence.

(These are widely used inside Google for reasons that I believe are mostly peculiar to Google's own internal libraries and code base.)

@jeffreydwalter
Copy link
Author

@jeffreydwalter jeffreydwalter commented Jan 31, 2019

I know this is old, tired, and closed, but just did a quick search and as of today there are 196 references to UnixMilli call in github today and 133 references to UnixMicro on github in public repos.

Not a crazy #, but still...

@jeffreydwalter
Copy link
Author

@jeffreydwalter jeffreydwalter commented Nov 21, 2019

I know this is closed, but... we're up to 306 references to UnixMilli and 180 references to UnixMicro on github in public repos.

Where's the bar?

@cespare
Copy link
Contributor

@cespare cespare commented Nov 21, 2019

More data: our internal time package has UnixMilli and FromUnixMilli. I counted up the calls:

function count
time.Unix 39
time.Time.Unix 108
time.Time.UnixNano 51
timeutil.UnixMilli 281
timeutil.FromUnixMilli 128

So we want milliseconds much more often than we want seconds or nanoseconds.

@cespare
Copy link
Contributor

@cespare cespare commented Nov 25, 2019

By the way, @rsc, you wrote

As already noted, if a third-party package becomes very widely used then we'll consider lifting them into package time. But I haven't seen any new evidence.

I don't think that's a good test here. I explained in my previous comment that our auxiliary functions are indeed widely used internally at work. However, I wouldn't use some third-party package to provide UnixMilli/FromUnixMilli in open-source work. In fact, I consider pulling in dependencies to provide such trivial functions to be something of a code smell. If I need unix millis in open-source code, I write t.UnixNano() / 1e6.

@justinhwang
Copy link

@justinhwang justinhwang commented Feb 29, 2020

Likewise @cespare, I see similar logic duplicated throughout in my work and reimplemented in internal "utilities" packages. I was going to open another proposal thread until I found the existing conversation--I don't think that the usage of a single third-party package is a good measure of whether this should be natively supported as most people are probably creating some sort of util file and putting it in there

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

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.