Skip to content

Switch default time format for ingest from Joda to Java for v7#37934

Merged
droberts195 merged 3 commits intoelastic:masterfrom
droberts195:java_time_default_for_ingest_in_7
Jan 30, 2019
Merged

Switch default time format for ingest from Joda to Java for v7#37934
droberts195 merged 3 commits intoelastic:masterfrom
droberts195:java_time_default_for_ingest_in_7

Conversation

@droberts195
Copy link
Copy Markdown

@droberts195 droberts195 commented Jan 28, 2019

Date formats with and without the "8" prefix are now all treated
as Java time formats, so that ingest does the same as mappings
in this respect.

Relates #27330

Date formats with and without the "8" prefix are now all treated
as Java time formats, so that ingest does the same as mappings
in this respect.
@droberts195 droberts195 added :Core/Infra/Core Core issues without another label v7.0.0 labels Jan 28, 2019
@droberts195 droberts195 requested a review from spinscale January 28, 2019 16:43
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra

@droberts195
Copy link
Copy Markdown
Author

I opened issue #37935 to discuss side effects of the fact that this change also changes the formats used for date index naming.

Copy link
Copy Markdown
Contributor

@spinscale spinscale left a comment

Choose a reason for hiding this comment

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

I left one comment, as we need to do some more work here, but I can just do that as well.

ZonedDateTime defaultZonedDateTime = Instant.EPOCH.atZone(ZoneOffset.UTC).withYear(year);
TemporalAccessor accessor = formatter.parse(text);
long millis = DateFormatters.toZonedDateTime(accessor, defaultZonedDateTime).toInstant().toEpochMilli();
return new DateTime(millis, timezone);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we are still returned a joda time class here, that we need to get rid of. I am happy to fix this in a follow up PR though

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks @spinscale. I'd prefer to leave that for a followup PR as I'm sure removing Joda classes will spread out far and wide through the codebase.

@droberts195
Copy link
Copy Markdown
Author

Jenkins run elasticsearch-ci/1

@droberts195
Copy link
Copy Markdown
Author

I'll hold off from merging this until somebody has commented on #37935.

@droberts195 droberts195 merged commit 2f7776c into elastic:master Jan 30, 2019
@droberts195 droberts195 deleted the java_time_default_for_ingest_in_7 branch January 30, 2019 16:26
jasontedor added a commit to dnhatn/elasticsearch that referenced this pull request Jan 30, 2019
* master:
  Expose retention leases in shard stats (elastic#37991)
  Make primary terms fields private in index shard (elastic#38036)
  ML: Add reason field in JobTaskState (elastic#38029)
  Log flush_stats and commit_stats in testMaybeFlush
  HLRC: Fix strict setting exception handling (elastic#37247)
  Test: Enable strict deprecation on all tests (elastic#36558)
  Removes typed calls from YAML REST tests (elastic#37611)
  Switch default time format for ingest from Joda to Java for v7 (elastic#37934)
  Remove deprecated Plugin#onModule extension points (elastic#37866)
  Geo: Fix Empty Geometry Collection Handling (elastic#37978)
@jimczi jimczi added :Distributed/Ingest Node Execution or management of Ingest Pipelines and removed :Core/Infra/Core Core issues without another label labels Feb 11, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-features

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

Labels

:Distributed/Ingest Node Execution or management of Ingest Pipelines >non-issue v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants