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

[Refactor] rebase to support latest core snapshot #510

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

nknize
Copy link
Contributor

@nknize nknize commented Aug 8, 2023

This PR refactors the codebase to comply with latest breaking changes on the core repository. It also updates gradle to the same version as core (currently 8.2.1) and adds initial support for spotless plugin.

This commit refactors the codebase to comply with latest breaking
changes on the core repository.

Signed-off-by: Nicholas Walter Knize <[email protected]>
@nknize
Copy link
Contributor Author

nknize commented Aug 8, 2023

/cc @sbcd90 @getsaurabh02

@nknize
Copy link
Contributor Author

nknize commented Aug 8, 2023

I'm not sure spotlessApply is running like I'd expect. Maybe we should check w/ @peternied if the security team was able to get it working w/ the security repo.

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #510 (4e9a15d) into main (9a3213b) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               main     #510      +/-   ##
============================================
- Coverage     28.20%   28.17%   -0.04%     
+ Complexity      941      939       -2     
============================================
  Files           236      236              
  Lines          9864     9864              
  Branches       1118     1118              
============================================
- Hits           2782     2779       -3     
- Misses         6841     6842       +1     
- Partials        241      243       +2     
Files Changed Coverage Δ
...rch/securityanalytics/SecurityAnalyticsPlugin.java 2.56% <ø> (ø)
...rch/securityanalytics/action/AckAlertsRequest.java 75.00% <ø> (ø)
...ch/securityanalytics/action/AckAlertsResponse.java 77.27% <ø> (ø)
.../opensearch/securityanalytics/action/AlertDto.java 50.45% <ø> (ø)
...rityanalytics/action/CorrelatedFindingRequest.java 0.00% <ø> (ø)
...ityanalytics/action/CorrelatedFindingResponse.java 0.00% <ø> (ø)
...tyanalytics/action/CreateIndexMappingsRequest.java 46.93% <0.00%> (ø)
...analytics/action/DeleteCorrelationRuleRequest.java 0.00% <ø> (ø)
...ecurityanalytics/action/DeleteDetectorRequest.java 0.00% <ø> (ø)
...curityanalytics/action/DeleteDetectorResponse.java 0.00% <ø> (ø)
... and 86 more

Copy link
Collaborator

@sbcd90 sbcd90 left a comment

Choose a reason for hiding this comment

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

thanks a lot @nknize .

@peternied
Copy link
Member

@nknize Hmm, are you expecting to have the formatter run when running gradlew assemble?

In the security repo we don't have any task dependency to it in our security plugin, instead we have separate code hygiene workflows that trigger the task.

Confirmed this was the case for the security plugin by using gradle-task-tree

./gradlew spotlessCheck taskTree 
...
:spotlessCheck
+--- :spotlessJavaCheck
|    \--- :spotlessJava
|         \--- :spotlessInternalRegisterDependencies
\--- :spotlessMiscCheck
     \--- :spotlessMisc
          \--- :spotlessInternalRegisterDependencies *

Copy link
Member

@getsaurabh02 getsaurabh02 left a comment

Choose a reason for hiding this comment

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

security tests are failing since docker image are not up to date.

@sbcd90 sbcd90 merged commit 6d49245 into opensearch-project:main Aug 8, 2023
@sbcd90
Copy link
Collaborator

sbcd90 commented Aug 8, 2023

hi @peternied , will look into it.

lezzago pushed a commit to lezzago/security-analytics that referenced this pull request Aug 24, 2023
…#510)

This commit refactors the codebase to comply with latest breaking
changes on the core repository.

Signed-off-by: Nicholas Walter Knize <[email protected]>
@amsiglan amsiglan mentioned this pull request Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants