-
Notifications
You must be signed in to change notification settings - Fork 207
jackson ObjectMapper
: auto-detect modules
#1643
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
jackson ObjectMapper
: auto-detect modules
#1643
Conversation
9ca9142
to
2a17b0e
Compare
the link checker failure is unrelated: |
java-client/src/main/java/org/opensearch/client/json/jackson/JacksonJsonpMapper.java
Show resolved
Hide resolved
LGTM, one minor comment, thanks @rursprung |
f164c43
to
05a8862
Compare
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com> [PR adding JSR310 date support]: FasterXML/jackson-databind#5032
05a8862
to
d0a398e
Compare
@Xtansia no concerns from you? thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No concerns from me, thanks @rursprung
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR #251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com> [PR adding JSR310 date support]: FasterXML/jackson-databind#5032 (cherry picked from commit c1ae512) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR #251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). [PR adding JSR310 date support]: FasterXML/jackson-databind#5032 (cherry picked from commit c1ae512) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
thanks everyone! can we backport this to 2.x as well? |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/opensearch-java/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/opensearch-java/backport-2.x
# Create a new branch
git switch --create backport/backport-1643-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c1ae512803223204800a860f797f146871131bda
# Push it to GitHub
git push --set-upstream origin backport/backport-1643-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/opensearch-java/backport-2.x Then, create a pull request where the |
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com> [PR adding JSR310 date support]: FasterXML/jackson-databind#5032 (cherry picked from commit c1ae512) Signed-off-by: Thomas Farr <tsfarr@amazon.com>
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com> [PR adding JSR310 date support]: FasterXML/jackson-databind#5032 (cherry picked from commit c1ae512)
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR #251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). [PR adding JSR310 date support]: FasterXML/jackson-databind#5032 (cherry picked from commit c1ae512) Signed-off-by: Thomas Farr <tsfarr@amazon.com> Co-authored-by: Ralph Ursprung <39383228+rursprung@users.noreply.github.com>
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com> [PR adding JSR310 date support]: FasterXML/jackson-databind#5032 Signed-off-by: psingh3 <pushpendra.singh@walmart.com>
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com> [PR adding JSR310 date support]: FasterXML/jackson-databind#5032 Signed-off-by: psingh3 <pushpendra.singh@walmart.com>
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this [PR adding JSR310 date support] to 3.x). PR opensearch-project#251 added documentation on how a custom `ObjectMapper` can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the same `ObjectMapper` (which in turn the docs mention as best practices). Signed-off-by: Ralph Ursprung <Ralph.Ursprung@avaloq.com> [PR adding JSR310 date support]: FasterXML/jackson-databind#5032 Signed-off-by: psingh3 <pushpendra.singh@walmart.com>
jackson 2.x still supports java 7 and thus does not automatically support java 8 classes. this will change with jackson 3.x (see e.g. this PR adding JSR310 date support to 3.x).
PR #251 added documentation on how a custom
ObjectMapper
can be registered - but there's no reason why we can't just enable auto-detection centrally. the docs mention that performance should be considered, but this is IMHO not relevant here since we only construct it once and then keep the sameObjectMapper
(which in turn the docs mention as best practices).Signed-off-by: Ralph Ursprung Ralph.Ursprung@avaloq.com
this is split from #1614. CC @reta & @Xtansia
Description
Describe what this change achieves.
Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.