Skip to content

Remove JodaCompatibleZonedDateTime#78417

Merged
jdconrad merged 10 commits intoelastic:masterfrom
jdconrad:remjoda
Sep 29, 2021
Merged

Remove JodaCompatibleZonedDateTime#78417
jdconrad merged 10 commits intoelastic:masterfrom
jdconrad:remjoda

Conversation

@jdconrad
Copy link
Copy Markdown
Contributor

@jdconrad jdconrad commented Sep 28, 2021 •
edited
Loading

This change removes JodaCompatibleZonedDateTime and replaces it with ZonedDateTime for use in scripting.

Breaking changes:

  1. JodaCompatibleDateTime no longer exists and cannot be cast to in Painless. Use ZonedDateTime instead.
  2. The dayOfWeek method on ZonedDateTime returns the DayOfWeek enum instead of an int from JodaCompatibleDateTime. dayOfWeekEnum still exists on ZonedDateTime as an augmentation to support the transition to ZonedDateTime, but is now deprecated in favor of dayOfWeek on ZonedDateTime

@jdconrad jdconrad requested a review from rjernst September 28, 2021 21:06
@elasticmachine elasticmachine added Team:Core/Infra Meta label for core/infra team Team:Data Management (obsolete) DO NOT USE. This team no longer exists. labels Sep 28, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Sep 28, 2021

Pinging @elastic/es-data-management (Team:Data Management)

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Sep 28, 2021

Pinging @elastic/es-core-infra (Team:Core/Infra)

@jdconrad
Copy link
Copy Markdown
Contributor Author

jdconrad commented Sep 28, 2021

@elasticmachine run elasticsearch-ci/part-2

@jdconrad jdconrad removed the WIP label Sep 29, 2021
rjernst
rjernst approved these changes Sep 29, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM!

@jdconrad jdconrad requested a review from bpintea September 29, 2021 17:21
@jdconrad jdconrad requested a review from jrodewig September 29, 2021 17:52
@jdconrad
Copy link
Copy Markdown
Contributor Author

jdconrad commented Sep 29, 2021

@jrodewig Please take a look at the breaking change documentation when you have a chance. Thanks!

jrodewig
jrodewig approved these changes Sep 29, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jrodewig jrodewig left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The docs look good overall. However, I'd explain what will happen if a script uses one of the outdated methods in the impact section of the breaking change.

bpintea
bpintea approved these changes Sep 29, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@bpintea bpintea left a comment

Choose a reason for hiding this comment

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

LGTM

@jdconrad
Copy link
Copy Markdown
Contributor Author

jdconrad commented Sep 29, 2021

@rjernst @bpintea @jrodewig Thanks for the reviews! Will commit as soon as CI passes.

@jdconrad jdconrad merged commit 086ba1a into elastic:master Sep 29, 2021
@stu-elastic stu-elastic mentioned this pull request Oct 4, 2021
Merged
@majagrubic majagrubic mentioned this pull request Oct 6, 2021
Closed
@jdconrad jdconrad mentioned this pull request Oct 18, 2021
Closed
sakurai-youhei added a commit to sakurai-youhei/elasticsearch that referenced this pull request Jul 10, 2022
@sakurai-youhei
Add conditional branch for JodaCompatibleZonedDateTime
58d57be 
v1 can be an instance of JodaCompatibleZonedDateTime in versions prior
to elastic#78417, which has to be converted to ZonedDateTime for appropriate
comparison.

Closes #TBC
sakurai-youhei added a commit to sakurai-youhei/elasticsearch that referenced this pull request Jul 12, 2022
@sakurai-youhei
Add conditional branch for JodaCompatibleZonedDateTime
df128f1 
v1 can be an instance of JodaCompatibleZonedDateTime in versions prior
to elastic#78417, which has to be converted to ZonedDateTime for appropriate
comparison.

Closes elastic#88408
stu-elastic pushed a commit that referenced this pull request Jul 13, 2022
@sakurai-youhei
[7.17] Adjust date conversion for execution context in watcher compar…
0f64f22 
…e condition (#88467)

* Add test of date math with ctx.execution_time

Add one test to confirm the below watcher compare condition is always
met.

```
  "condition": {
    "compare": {
      "ctx.execution_time": {
        "gte": "<{now-5m}>"
      }
    }
  }
```

v1 can be an instance of JodaCompatibleZonedDateTime in versions prior
to #78417, which has to be converted to ZonedDateTime for appropriate
comparison.

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

Reviewers

@rjernst rjernst rjernst approved these changes

@bpintea bpintea bpintea approved these changes

+1 more reviewer

@jrodewig jrodewig jrodewig approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

>breaking :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache :Distributed/Watcher >refactoring Team:Core/Infra Meta label for core/infra team Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v8.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jdconrad @elasticmachine @rjernst @bpintea @jrodewig @jakelandis @stu-elastic