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

Fix the import org.opensearch.core.common.Strings; and import org.opensearch.core.common.logging.LoggerMessageFormat; #2781

Merged
merged 1 commit into from
May 23, 2023

Conversation

RyanL1997
Copy link
Collaborator

@RyanL1997 RyanL1997 commented May 18, 2023

Description

Fix the import org.opensearch.core.common.Strings; and import org.opensearch.core.common.logging.LoggerMessageFormat; to adapt the breaking change in core: opensearch-project/OpenSearch#7508

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
    Refactoring

Issues Resolved

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed 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.

@@ -463,15 +463,15 @@ public String getDocId() {
@Override
public String toString() {
try {
return Strings.toString(JsonXContent.contentBuilder().map(getAsMap()));
return org.opensearch.common.Strings.toString(JsonXContent.contentBuilder().map(getAsMap()));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NOTE: this line and L474 are temporarily changed into this since the original toString() function is undefined in org.opensearch.core.common.Strings. I will create an issue to track that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Issue I created: #2782

@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #2781 (a68e734) into main (15860b6) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #2781      +/-   ##
============================================
- Coverage     61.48%   61.47%   -0.02%     
+ Complexity     3402     3401       -1     
============================================
  Files           266      266              
  Lines         18865    18865              
  Branches       3302     3302              
============================================
- Hits          11600    11597       -3     
- Misses         5669     5671       +2     
- Partials       1596     1597       +1     
Impacted Files Coverage Δ
...ic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java 55.29% <ø> (ø)
...ic/auth/ldap/backend/LDAPAuthorizationBackend.java 57.81% <ø> (ø)
...zon/dlic/auth/ldap2/LDAPAuthorizationBackend2.java 31.48% <ø> (ø)
...opensearch/security/auditlog/sink/WebhookSink.java 76.74% <ø> (ø)
...ensearch/security/compliance/ComplianceConfig.java 83.33% <ø> (ø)
...earch/security/dlic/rest/api/AccountApiAction.java 81.15% <ø> (ø)
...rity/dlic/rest/api/PatchableResourceApiAction.java 85.60% <ø> (ø)
...ity/dlic/rest/validation/CredentialsValidator.java 70.00% <ø> (ø)
...curity/dlic/rest/validation/PasswordValidator.java 84.05% <ø> (ø)
...org/opensearch/security/filter/SecurityFilter.java 71.75% <ø> (ø)
... and 8 more

... and 1 file with indirect coverage changes

@DarshitChanpura
Copy link
Member

Plugin-install CI task will pass once common-utils artifact is available with opensearch-project/common-utils#432 merged

@RyanL1997
Copy link
Collaborator Author

Checking up the errors in plugin-install

@RyanL1997 RyanL1997 changed the title Fix the import org.opensearch.core.common.Strings; Fix the import org.opensearch.core.common.Strings; and import org.opensearch.core.common.logging.LoggerMessageFormat; May 19, 2023
@cwperks
Copy link
Member

cwperks commented May 22, 2023

@opensearch-project/engineering-effectiveness can a build be triggered to produce new artifacts for common-utils? The CI for this build fixing PR is blocked by stale artifacts.

@prudhvigodithi
Copy link
Member

@opensearch-project/engineering-effectiveness can a build be triggered to produce new artifacts for common-utils? The CI for this build fixing PR is blocked by stale artifacts.

I have triggered one with just OpenSearch common-utils.

@RyanL1997
Copy link
Collaborator Author

I have triggered one with just OpenSearch common-utils.

Hi @prudhvigodithi, thank you for the follow-up! However, the new build seems failed (https://build.ci.opensearch.org/job/distribution-build-opensearch/7870/console) :/

@prudhvigodithi
Copy link
Member

I have triggered one with just OpenSearch common-utils.

Hi @prudhvigodithi, thank you for the follow-up! However, the new build seems failed (https://build.ci.opensearch.org/job/distribution-build-opensearch/7870/console) :/

Ya, I have just noticed it failed with OpenSearch :( and did not even reach common-utils, there is already an open issue opensearch-project/OpenSearch#7590 in OpenSearch repo.

@RyanL1997
Copy link
Collaborator Author

@prudhvigodithi got it, thanks! I will keep tracking that.

@cwperks cwperks merged commit 33aebb9 into opensearch-project:main May 23, 2023
MaciejMierzwa pushed a commit to MaciejMierzwa/security that referenced this pull request Jun 13, 2023
MaciejMierzwa pushed a commit to MaciejMierzwa/security that referenced this pull request Jun 13, 2023
samuelcostae pushed a commit to samuelcostae/security that referenced this pull request Jun 19, 2023
samuelcostae pushed a commit to samuelcostae/security that referenced this pull request Jun 19, 2023
@cwperks cwperks added the backport 2.x backport to 2.x branch label Jul 7, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-2781-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 33aebb96781a4988ae14fab2a5e53b17bb99ecf6
# Push it to GitHub
git push --set-upstream origin backport/backport-2781-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-2781-to-2.x.

@cwperks
Copy link
Member

cwperks commented Jul 7, 2023

Creating a manual backport now

@cwperks
Copy link
Member

cwperks commented Jul 7, 2023

Manual backport created: #2953

cwperks added a commit that referenced this pull request Jul 7, 2023
… import org.opensearch.core.common.logging.LoggerMessageFormat; (#2953)

* Use version of httpclient 4 from core's buildSrc/version.properties

Signed-off-by: Craig Perkins <[email protected]>

* Fix the `import org.opensearch.core.common.Strings;` (#2781)

Signed-off-by: Ryan Liang <[email protected]>
(cherry picked from commit 33aebb9)

* Run spotlessApply

Signed-off-by: Craig Perkins <[email protected]>

* Set common-utils-version to 2.9.0.0-SNAPSHOT

Signed-off-by: Craig Perkins <[email protected]>

---------

Signed-off-by: Craig Perkins <[email protected]>
Co-authored-by: Ryan Liang <[email protected]>
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
None yet
Development

Successfully merging this pull request may close these issues.

5 participants