Skip to content

Remove JodaCompatibleZonedDateTime #78417

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Sep 29, 2021
Merged

Remove JodaCompatibleZonedDateTime #78417

merged 10 commits into from
Sep 29, 2021

Conversation

jdconrad
Copy link
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 Meta label for data/management team labels Sep 28, 2021
@elasticmachine
Copy link
Collaborator

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

@elasticmachine
Copy link
Collaborator

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

@jdconrad
Copy link
Contributor Author

@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
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
Contributor Author

@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
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
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
Contributor Author

@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

@jrodewig jrodewig jrodewig approved these changes

Assignees
No one assigned
Labels
>breaking :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache :Data Management/Watcher >refactoring Team:Core/Infra Meta label for core/infra team Team:Data Management Meta label for data/management team v8.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jdconrad @elasticmachine @rjernst @bpintea @jrodewig @jakelandis