Skip to content
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

[BUG] Remove jackson-core dependency #211

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

sejli
Copy link
Collaborator

@sejli sejli commented Nov 22, 2023

Description

I noticed that we had a nightly build failure with conflicting jackson-core dependencies. Since jackson-core was bumped to 2.16.0 in core, our dependency in both processors' build.gradle's are conflicting. This PR reintroduces the question, should we remove the jackson-core dependency in the plugin's build.gradle, or keep bumping the version when it's bumped in core?

As discussed in the OpenSearch Search Relevance public meeting, bumping the version to match core seems to be a safer option since previously, there have been issues with removing the dependencies altogether.

Issues Resolved

#133

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed as per the DCO using --signoff

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.

Signed-off-by: Sean Li <lnse@amazon.com>
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b19f8c9) 83.30% compared to head (d7f8ffd) 83.30%.

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #211   +/-   ##
=========================================
  Coverage     83.30%   83.30%           
  Complexity      337      337           
=========================================
  Files            43       43           
  Lines          1270     1270           
  Branches        155      155           
=========================================
  Hits           1058     1058           
  Misses          134      134           
  Partials         78       78           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Sean Li <lnse@amazon.com>
Signed-off-by: Sean Li <lnse@amazon.com>
Copy link
Collaborator

@noCharger noCharger left a comment

Choose a reason for hiding this comment

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

Approved with comments: we should prioritize on #133

@noCharger noCharger merged commit f7570dd into opensearch-project:main Dec 6, 2023
16 checks passed
@sejli sejli added the backport 2.x Backport to 2.x branch label Dec 6, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Dec 6, 2023
* removing jackson core dependency

Signed-off-by: Sean Li <lnse@amazon.com>

* readding jackson core and bumping version

Signed-off-by: Sean Li <lnse@amazon.com>

* bumping versions for jackson databind and annotations

Signed-off-by: Sean Li <lnse@amazon.com>

---------

Signed-off-by: Sean Li <lnse@amazon.com>
(cherry picked from commit f7570dd)
sejli added a commit that referenced this pull request Dec 6, 2023
* removing jackson core dependency

Signed-off-by: Sean Li <lnse@amazon.com>

* readding jackson core and bumping version

Signed-off-by: Sean Li <lnse@amazon.com>

* bumping versions for jackson databind and annotations

Signed-off-by: Sean Li <lnse@amazon.com>

---------

Signed-off-by: Sean Li <lnse@amazon.com>
(cherry picked from commit f7570dd)

Co-authored-by: Sean Li <lnse@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants