-
Notifications
You must be signed in to change notification settings - Fork 356
Migrate SCANOSS implementation to use SCANOSS SDK #10265
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
Migrate SCANOSS implementation to use SCANOSS SDK #10265
Conversation
Remove the path anonymization functionality from the existing SCANOSS implementation as preparation for migrating to the Java SCANOSS SDK. This is a temporary removal. While path anonymization is not yet available in the SDK, we plan to implement this feature in the upstream SDK in the future. This approach allows us to consolidate all SCANOSS functionality in the SDK rather than maintaining custom implementations. Signed-off-by: Agustin Isasmendi <[email protected]>
Replace custom direct API calls to SCANOSS with the official Java SDK. This change improves maintainability by leveraging the SDK's functionality instead of maintaining custom implementation for API interactions. Signed-off-by: Agustin Isasmendi <[email protected]>
It forces the SCANOSS scanner's matcher property to null to prevent loading results from scan storage. This follows the same approach implemented for other snippet scanners, where the consensus was that snippet scanner results should never come from scan storage. This fixes an issue where `context.excludes` was being nullified in `ScanOss.scanPath()`, preventing proper application of exclusion patterns. Signed-off-by: Agustin Isasmendi <[email protected]>
| ) | ||
| ) | ||
| } | ||
| override val matcher: ScannerMatcher? = null |
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.
@isasmendiagus You should also change the property below to override val readFromStorage = false and remove the readFromStorage property from the config class, like it is also done in FossId.
I'm not requesting a change because you can also do it separately.
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.
I haven't seen that. I'll implement the change on PR #10287
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.
@isasmendiagus, please create a separate PR for this, as I want to get this in earlier than our discussion about the report generation PR will come to a conclusion.
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.
See #10291.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10265 +/- ##
============================================
+ Coverage 56.39% 56.46% +0.06%
- Complexity 1602 1603 +1
============================================
Files 331 331
Lines 12261 12261
Branches 1141 1141
============================================
+ Hits 6915 6923 +8
+ Misses 4897 4889 -8
Partials 449 449
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR migrates the SCANOSS integration from direct API calls to the official Java SDK.
Changes include: