Add option to _cat/indices to return index creation date #11524

[szroland]

Returning index creation date, both as a numeric millisecond value and
as a string. This implements #11524

[clintongormley]

Hi @szroland

Thanks for the PR. I note that no tests have been added or updated (specifically the REST tests). Please could you add.

thanks

[szroland]

Hi @clintongormley,

I added tests for the new fields to the default cat.indices test case.
Let me know if anything else is needed.

Cheers,
Roland

[nik9000]

Looks good to me.

[clintongormley]

@nik9000 want to merge it in?

[nik9000]

@clintongormley sure.

[nik9000]

@szroland, I'm getting test failures:

Suite: org.elasticsearch.test.rest.Rest0Tests
  2> REPRODUCE WITH: mvn test -Pdev -Dtests.seed=F28F8AF862FDDF5 -Dtests.class=org.elasticsearch.test.rest.Rest0Tests -Dtests.slow=true -Dtests.method="test {yaml=cat.indices/10_basic/Test cat indices output}" -Des.logger.level=ERROR -Dtests.assertion.disabled=false -Dtests.security.manager=true -Dtests.heap.size=512m -Dtests.locale=fi_FI -Dtests.timezone=Pacific/Auckland
FAILURE 0.14s | Rest0Tests.test {yaml=cat.indices/10_basic/Test cat indices output} <<<
   > Throwable #1: java.lang.AssertionError: field [$body] was expected to match the provided regex but didn't
   > Expected: ^(
   >    index1                                                    \s+
   >    (\d+)                                                     \s+
   >    (\d\d\d\d\-\d\d\-\d\d T\d\d:\d\d:\d\d.\d\d\d\+\d\d:\d\d)  \s+
   >    (\d+)                                                     \s+
   >    (\d\d\d\d\-\d\d\-\d\d T\d\d:\d\d:\d\d.\d\d\d\+\d\d:\d\d)  \s*
   >  )
   >  $
   >      but: was "index1 1438266964787 2015-07-30T10:36:04.787-04:00 1438266964787 2015-07-30T10:36:04.787-04:00 \n"

I'll do a bit of debugging and then let you know what I find. Maybe locale issues? Those times look like they are printed in a pretty ISO8601-ish format so I wouldn't expect them to vary based on locale but, you know, computers are tricky beasts.

[nik9000]

Ok - DateTime's toString is documented as spitting out its results in ISO8601 which doesn't have that space between the date and the T. It doesn't look locale dependent. So, I revise my review: doesn't look good to me, remove the space in those regexes and then it will.

[nik9000]

I've dropped the review tag and added feedback_needed so we can track that this is waiting another (tiny) patch. I don't think feedback_needed is really the right label but its the closest one I can find.

[szroland]

Interesting. Strictly speaking the error message makes sense since the date string does not contain that space. I did go ahead and remove it, also while at it brought the PR up in line with the latest commits.

Still, this test wasn't failing for me, since the rest tests do pattern matching using the Pattern.COMMENTS flag, which supposedly allows comments and ignores white space in the regular expression. This seems to be the case for me on Windows with both a 1.7 and 1.8 Oracle jvm. E.g. the test passes with or without that space. Out of curiosity, what platform / JVM did you run this test on?

[nik9000]

Out of curiosity, what platform / JVM did you run this test on?

Its OSX.

$ java -version
java version "1.8.0_51"
Java(TM) SE Runtime Environment (build 1.8.0_51-b16)
Java HotSpot(TM) 64-Bit Server VM (build 25.51-b03, mixed mode)

I'm not really sure what is up. I hadn't thought about the COMMENTS flag but its obvious from looking at the regexes it was on. I'll rerun the tests and recheck.

[nik9000]

/me shakes fist at time zones. Its because you are in a GMT + and I'm in a GMT -. It wasn't the space. One moment.

[nik9000]

@szroland I sent you a pull request for the branch you've used for this pull request. If you merge it it should fix the test for me and it'll become part of this pull request.

[szroland]

Ah right, I didn't notice that either. Makes more sense than having weird consistency issues with the behavior of Pattern.COMMENTS. Merged your PR.

[nik9000]

OK! That looks great and everything passes now so I've merged it. Thanks @szroland!