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

[ML] Adjust structure finder for Joda to Java time migration #37306

Conversation

Projects
None yet
4 participants
@droberts195
Copy link
Contributor

commented Jan 10, 2019

The ML file structure finder has always reported both Joda
and Java time format strings. This change makes the Java time
format strings the ones that are incorporated into mappings
and ingest pipeline definitions.

The BWC syntax of prepending "8" to these formats is used.
This will need to be removed once Java time format strings
become the default in Elasticsearch.

This commit also removes direct imports of Joda classes in the
structure finder unit tests. Instead the core Joda BWC class
is used.

[ML] Adjust structure finder for Joda to Java time migration
The ML file structure finder has always reported both Joda
and Java time format strings.  This change makes the Java time
format strings the ones that are incorporated into mappings
and ingest pipeline definitions.

The BWC syntax of prepending "8" to these formats is used.
This will need to be removed once Java time format strings
become the default in Elasticsearch.

This commit also removes direct imports of Joda classes in the
structure finder unit tests.  Instead the core Joda BWC class
is used.
@elasticmachine

This comment has been minimized.

Copy link

commented Jan 10, 2019

@droberts195

This comment has been minimized.

Copy link
Contributor Author

commented Jan 10, 2019

This PR should not be merged until Ingest pipeline has been changed to accept Java time format strings. The necessary changes are being made in #37407. I will double check the changes in this PR once #37407 is merged.

@droberts195

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2019

#37407 is now merged and I have updated this PR to account for it.

@benwtrent

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

For my edification: why are some formats prefixed with a 8?

@droberts195

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2019

For my edification: why are some formats prefixed with a 8?

The 8 prefix is a way of telling the ES mappings and ingest code that the format is a Java time format rather than a Joda time format during the transition period.

In ES version 6.7 Joda time formats are the default and Java time formats must be prefixed with 8.

In ES version 7.x Joda time formats will no longer be supported in ES but the 8 prefix will be optional for Java time formats for BWC. Closer to the 7.0 beta1 feature freeze there will be more changes to the ES time code to implement this, and I will have to raise another PR to modify the file structure finder again. (That future PR will be for 7.0 only.)

@droberts195 droberts195 merged commit f2c0c26 into elastic:master Jan 26, 2019

8 checks passed

CLA Commit author has signed the CLA
Details
elasticsearch-ci/1 Build finished.
Details
elasticsearch-ci/2 Build finished.
Details
elasticsearch-ci/default-distro Build finished.
Details
elasticsearch-ci/docbldesx Build finished.
Details
elasticsearch-ci/docs-check Build finished.
Details
elasticsearch-ci/oss-distro-docs Build finished.
Details
elasticsearch-ci/packaging-sample Build finished.
Details

@droberts195 droberts195 deleted the droberts195:use_joda_bwc_layer_in_ml_structure_finder branch Jan 26, 2019

droberts195 added a commit that referenced this pull request Jan 26, 2019

[ML] Adjust structure finder for Joda to Java time migration (#37306)
The ML file structure finder has always reported both Joda
and Java time format strings.  This change makes the Java time
format strings the ones that are incorporated into mappings
and ingest pipeline definitions.

The BWC syntax of prepending "8" to these formats is used.
This will need to be removed once Java time format strings
become the default in Elasticsearch.

This commit also removes direct imports of Joda classes in the
structure finder unit tests.  Instead the core Joda BWC class
is used.

@colings86 colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.