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

Remove joda date deprecation message on startup in 6.8 #45484

Merged
merged 2 commits into from
Aug 13, 2019

Conversation

spinscale
Copy link
Contributor

When starting up Elasticsearch 6.8, there is always a deprecation
message logged like this

[WARN ][o.e.d.c.j.Joda ] [A2Wr_Jx] 'y' year should be replaced with 'u'. Use 'y' for year-of-era. Prefix your date format with '8' to use the new specifier.

tracing this down, result in the LicenseService being the culprit using
an deprecated date. This switches to a java time based date.

Note: master/7.x/7.3 branches use yyyy in java time and not joda time, but as this is year-of-era, that might be fine for licensing.

Closes #45483

When starting up Elasticsearch 6.8, there is always a deprecation
message logged like this

[WARN ][o.e.d.c.j.Joda           ] [A2Wr_Jx] 'y' year should be replaced with 'u'. Use 'y' for year-of-era. Prefix your date format with '8' to use the new specifier.

tracing this down, result in the LicenseService being the culprit using
an deprecated date. This switches to a java time based date.
@henningandersen henningandersen added the :Core/Infra/Core Core issues without another label label Aug 13, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@pgomulka pgomulka left a comment

Choose a reason for hiding this comment

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

LGTM, but maybe we should use yyyy in 6.8? It don't think we necessarily need u here.

@spinscale
Copy link
Contributor Author

you mean for consistency among the 7.x implementation? that makes sense.

@spinscale spinscale merged commit 9d6b75e into elastic:6.8 Aug 13, 2019
@rjernst
Copy link
Member

rjernst commented Aug 13, 2019

y is specifically for year of era, meaning the format should contain era. I think we should stick with u.

@pgomulka
Copy link
Contributor

But isn't era defaulted from chronology? Don't have a doc handy but I think i have seen this mentioned

@pgomulka
Copy link
Contributor

I see your point in terms of resolve style. Era will be defaulted only in lenient or smart style. Not sure if this is a problem when parsing /formatting though. Even in strict mode.

https://docs.oracle.com/javase/8/docs/api/java/time/chrono/AbstractChronology.html#resolveDate-java.util.Map-java.time.format.ResolverStyle-

@richiejarvis
Copy link

@spinscale @pgomulka What is the feeling regarding whether the format change is/is not actually needed for the common y to u situation?

Folks are seeing it in their deprecation logs, and are wondering whether they really need to change it. My understanding (and please correct me if I am wrong) is that using YEAR_OF_ERA will not actually make any difference unless in certain exacting situations. Japanese Calendar or pre-1AD dates were the 2 I found. Especially as using the parse features of the Formatter should mean no difference in behaviour iirc.

Thanks in advance,

Richie

@pgomulka
Copy link
Contributor

pgomulka commented Nov 26, 2019

Hey @richiejarvis , most of the time it is safe as we made this a bit more flexible and allow using year_of_era in similar way as year in elasticsearch. We have enabled Strict date resolving, but still allow year_of_era to be passed without an era. see #46675

I would however advice to update this if possible.

I will be working on an issue that would make this upgrade more smooth #45548 This is high on my list and will look into this soon.

Also see if support support help page might be useful https://github.com/elastic/support-knowledge-articles/blob/master/Knowledge%20Share/Elasticsearch%20Joda%20-%20Java%20migration.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >non-issue v6.8.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants