[ML] ScheduledEvent takes and stores ZonedDateTime but then discards the timezone

[droberts195]

(Noticed while fixing #38506)

The ScheduledEvent class's constructor takes ZonedDateTime for both startTime and endTime. However, when serialising both on the wire and as XContent it ditches the time zone and converts to an Instant. It would make more sense to store Instant, take Instants in the constructor and return Instants from the accessors. Then it would be completely clear to all users that the class was not preserving the time zone.

[elasticmachine]

Pinging @elastic/ml-core

[Mehrank64]

Hello,
I started working on this issue. To be honest this is my first experience in contributing to open source projects and struggling to fully set-up the environment with all the details.
I did change what you described in the ticket my changes passed all unit tests, but have a problem with Integration test.

I am getting this error when trying to run ScheduledEventsIT

lang.IllegalArgumentException: transport type setting [transport.type] must be [security4] or [security-nio] but is [mock-nio]

at org.elasticsearch.xpack.core.security.SecuritySettings.addTransportSettings(SecuritySettings.java:23)
at org.elasticsearch.xpack.core.XPackClientPlugin.additionalSettings(XPackClientPlugin.java:235)
at org.elasticsearch.xpack.core.XPackClientPlugin.additionalSettings(XPackClientPlugin.java:229)
at org.elasticsearch.plugins.PluginsService.updatedSettings(PluginsService.java:210)
at org.elasticsearch.client.transport.TransportClient.buildTemplate(TransportClient.java:138)
at org.elasticsearch.client.transport.TransportClient.<init>(TransportClient.java:287)
at org.elasticsearch.transport.MockTransportClient.<init>(MockTransportClient.java:47)
at org.elasticsearch.transport.MockTransportClient.<init>(MockTransportClient.java:43)
at org.elasticsearch.test.InternalTestCluster$TransportClientFactory.client(InternalTestCluster.java:1101)
at org.elasticsearch.test.InternalTestCluster$NodeAndClient.getOrBuildTransportClient(InternalTestCluster.java:945)
at org.elasticsearch.test.InternalTestCluster$NodeAndClient.client(InternalTestCluster.java:902)
at org.elasticsearch.test.InternalTestCluster.client(InternalTestCluster.java:754)
at org.elasticsearch.test.TestCluster.wipeAllTemplates(TestCluster.java:167)
at org.elasticsearch.test.TestCluster.wipe(TestCluster.java:80)
at org.elasticsearch.test.ESIntegTestCase.lambda$beforeInternal$0(ESIntegTestCase.java:377)
at com.carrotsearch.randomizedtesting.RandomizedContext.runWithPrivateRandomness(RandomizedContext.java:187)
at com.carrotsearch.randomizedtesting.RandomizedContext.runWithPrivateRandomness(RandomizedContext.java:211)
at org.elasticsearch.test.ESIntegTestCase.beforeInternal(ESIntegTestCase.java:385)
at org.elasticsearch.test.ESIntegTestCase.setupTestCluster(ESIntegTestCase.java:2174)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1750)
at com.carrotsearch.randomizedtesting.RandomizedRunner$9.evaluate(RandomizedRunner.java:972)
at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:988)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at org.apache.lucene.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:49)
at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:45)
at org.apache.lucene.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:48)
at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:817)
at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:468)
at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:947)
at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:832)
at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:883)
at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:894)
at org.apache.lucene.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:45)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at org.apache.lucene.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:41)
at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at org.apache.lucene.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
at org.apache.lucene.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:47)
at org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:64)
at org.apache.lucene.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:54)
at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
at java.base/java.lang.Thread.run(Thread.java:834)  

I did the required setup based on the documentation in the project but didn't find anything regarding setup for integration tests. Can you help me please?

[droberts195]

Thanks for looking at this @Mehrank64. How did you run ScheduledEventsIT? Did you try to run it from your IDE? This is not possible for integration tests that use an external cluster.

Try running this from the top level of your clone of the repo:

./gradlew :x-pack:plugin:ml:qa:native-multi-node-tests:integTest -Dtests.class=org.elasticsearch.xpack.ml.integration.ScheduledEventsIT

Does the test pass on your machine when run like that? If not, how about on a clean clone without your changes? If it doesn't pass on a clean clone, what is the exact error you get from this gradlew command?

[Mehrank64]

@droberts195 Thanks for your help. The test passes when I run it with the above command

I did run the Unit test from the IDE and all should be fine now.

[droberts195]

That’s great @Mehrank64. Once the change is complete please open a PR to merge from the branch in your fork to master and @ mention me in it so I get notified.

[Mehrank64]

@droberts195 just did. let me know if you can't see it/any amendment is required