Skip to content

Fix compilation warning in CollectionUtils affecting maven snapshot publication#9149

Merged
dbwiddis merged 2 commits intoopensearch-project:mainfrom
cwperks:fix-publish-maven-snapshots
Aug 7, 2023
Merged

Fix compilation warning in CollectionUtils affecting maven snapshot publication#9149
dbwiddis merged 2 commits intoopensearch-project:mainfrom
cwperks:fix-publish-maven-snapshots

Conversation

@cwperks
Copy link
Copy Markdown
Member

@cwperks cwperks commented Aug 7, 2023

Description

Maven snapshot publication started failing after: #9120

The error stemmed from a compilation warning:

/home/runner/work/OpenSearch/OpenSearch/libs/core/src/main/java/org/opensearch/core/common/util/CollectionUtils.java:143: warning: [unchecked] unchecked generic array creation for varargs parameter of type Iterator<? extends Object>[]
return () -> Iterators.concat(map.keySet().iterator(), map.values().iterator());
^
error: warnings found and -Werror specified

See: https://github.com/opensearch-project/OpenSearch/actions/runs/5767554873/job/15637408847

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@cwperks
Fix compilation warning in CollectionUtils affecting maven snapshot p…
4ec3ab3 
…ublication

Signed-off-by: Craig Perkins <cwperx@amazon.com>
@cwperks
Copy link
Copy Markdown
Member Author

cwperks commented Aug 7, 2023

@nknize @reta Can I get a review on this? The maven snapshots stopped publishing after a change in CollectionUtils due to a compiler warning.

@cwperks cwperks mentioned this pull request Aug 7, 2023
Merged
3 tasks
@cwperks cwperks mentioned this pull request Aug 7, 2023
Merged
5 tasks
peternied
peternied approved these changes Aug 7, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Lets get this unblocked by merging,

  • Can we get a follow up issue to make sure warnings as errors is on by default in the build

@nknize
Copy link
Copy Markdown
Contributor

nknize commented Aug 7, 2023

  • Can we get a follow up issue to make sure warnings as errors is on by default in the build

💯 agree. I noticed this failed on 2.x but not main. That has a bad smell to it, as if the build configs are different. But we should be pushing to main and backporting so I'm not sure why main didn't fail w/o the suppress warning but 2.x did fail?

nknize
nknize suggested changes Aug 7, 2023
View reviewed changes
@cwperks
Use SuppressWarnings
d3a0780 
Signed-off-by: Craig Perkins <cwperx@amazon.com>
@opensearch-trigger-bot
Copy link
Copy Markdown
Contributor

opensearch-trigger-bot bot commented Aug 7, 2023

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/security-analytics.git]
Compatible components: [https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

BUILD SUCCESSFUL in 23m 46s

@cwperks cwperks mentioned this pull request Aug 7, 2023
Open
@cwperks
Copy link
Copy Markdown
Member Author

cwperks commented Aug 7, 2023

@peternied @nknize I filed an issue to investigate why the build on main did not catch this initially and see why there is a discrepancy between main and 2.x: #9150

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 7, 2023

Gradle Check (Jenkins) Run Completed with:

@opensearch-trigger-bot
Copy link
Copy Markdown
Contributor

opensearch-trigger-bot bot commented Aug 7, 2023

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/security-analytics.git]
Compatible components: [https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

BUILD SUCCESSFUL in 30m 46s

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 7, 2023

Gradle Check (Jenkins) Run Completed with:

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 7, 2023

Codecov Report

Merging #9149 (d3a0780) into main (bb78930) will decrease coverage by 0.07%.
Report is 4 commits behind head on main.
The diff coverage is 72.04%.

@@             Coverage Diff              @@
##               main    #9149      +/-   ##
============================================
- Coverage     71.06%   70.99%   -0.07%     
+ Complexity    57314    57280      -34     
============================================
  Files          4766     4765       -1     
  Lines        270453   270479      +26     
  Branches      39555    39553       -2     
============================================
- Hits         192197   192040     -157     
- Misses        62045    62256     +211     
+ Partials      16211    16183      -28
Files Changed Coverage Δ
.../opensearch/client/indices/CreateIndexRequest.java 77.77% <0.00%> (+1.11%) ⬆️
...opensearch/client/slm/SnapshotLifecyclePolicy.java 0.00% <ø> (ø)
...ch/client/slm/SnapshotLifecyclePolicyMetadata.java 0.00% <ø> (ø)
.../opensearch/client/slm/SnapshotLifecycleStats.java 0.00% <ø> (ø)
...rch/client/slm/SnapshotRetentionConfiguration.java 0.00% <ø> (ø)
...g/opensearch/core/common/util/CollectionUtils.java 77.06% <ø> (ø)
...h/script/mustache/MultiSearchTemplateResponse.java 59.01% <ø> (ø)
...rg/opensearch/index/rankeval/RankEvalResponse.java 97.14% <ø> (+7.14%) ⬆️
...va/org/opensearch/index/rankeval/RankEvalSpec.java 97.36% <ø> (ø)
...a/org/opensearch/index/rankeval/RatedDocument.java 81.03% <0.00%> (ø)
... and 135 more

... and 434 files with indirect coverage changes

@dbwiddis dbwiddis merged commit d603306 into opensearch-project:main Aug 7, 2023
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
@cwperks @kaushalmahi12
Fix compilation warning in CollectionUtils affecting maven snapshot p…
256d779 
…ublication (opensearch-project#9149)

* Fix compilation warning in CollectionUtils affecting maven snapshot publication

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Use SuppressWarnings

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
@cwperks @brusic
Fix compilation warning in CollectionUtils affecting maven snapshot p…
12deb8c 
…ublication (opensearch-project#9149)

* Fix compilation warning in CollectionUtils affecting maven snapshot publication

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Use SuppressWarnings

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
@cwperks @shiv0408
Fix compilation warning in CollectionUtils affecting maven snapshot p…
e238abe 
…ublication (opensearch-project#9149)

* Fix compilation warning in CollectionUtils affecting maven snapshot publication

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Use SuppressWarnings

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
@ThyTran1402 ThyTran1402 mentioned this pull request Mar 20, 2026
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peternied peternied peternied approved these changes

@dbwiddis dbwiddis dbwiddis approved these changes

@reta reta Awaiting requested review from reta

@anasalkouz anasalkouz Awaiting requested review from anasalkouz

@andrross andrross Awaiting requested review from andrross

@Bukhtawar Bukhtawar Awaiting requested review from Bukhtawar

@CEHENKLE CEHENKLE Awaiting requested review from CEHENKLE

@dblock dblock Awaiting requested review from dblock

@gbbafna gbbafna Awaiting requested review from gbbafna

@setiah setiah Awaiting requested review from setiah

@kartg kartg Awaiting requested review from kartg

@kotwanikunal kotwanikunal Awaiting requested review from kotwanikunal

@mch2 mch2 Awaiting requested review from mch2

@owaiskazi19 owaiskazi19 Awaiting requested review from owaiskazi19

@Rishikesh1159 Rishikesh1159 Awaiting requested review from Rishikesh1159

@ryanbogan ryanbogan Awaiting requested review from ryanbogan

@saratvemulapalli saratvemulapalli Awaiting requested review from saratvemulapalli

@shwetathareja shwetathareja Awaiting requested review from shwetathareja

@dreamer-89 dreamer-89 Awaiting requested review from dreamer-89

@tlfeng tlfeng Awaiting requested review from tlfeng

@VachaShah VachaShah Awaiting requested review from VachaShah

@sachinpkale sachinpkale Awaiting requested review from sachinpkale

@sohami sohami Awaiting requested review from sohami

+1 more reviewer

@nknize nknize nknize approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip-changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cwperks @nknize @peternied @dbwiddis