Fix issue with scan results from failed scans being reused even if scan problem is resolved #10475
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
lamppu
force-pushed
the
10054-fix-failed-scan-results-reused
branch
from
6b1b951 to
38b6494
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10475 +/- ##
=========================================
Coverage 56.75% 56.75%
+ Complexity 1644 1642 -2
=========================================
Files 337 337
Lines 12480 12480
Branches 1177 1177
=========================================
Hits 7083 7083
Misses 4945 4945
Partials 452 452
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:
|
sschuberth
previously requested changes
Jun 13, 2025
plugins/reporters/evaluated-model/src/main/kotlin/EvaluatedModelMapper.kt
Show resolved
Hide resolved
plugins/reporters/evaluated-model/src/main/kotlin/EvaluatedModelMapper.kt
Show resolved
Hide resolved
lamppu
force-pushed
the
10054-fix-failed-scan-results-reused
branch
from
38b6494 to
b2d6421
Compare
fviernau
previously requested changes
Jun 13, 2025
fviernau
dismissed
their stale review
change request not acceptable, because we must stick to previous decision.
lamppu
force-pushed
the
10054-fix-failed-scan-results-reused
branch
from
b2d6421 to
2d40d5b
Compare
mnonnenmacher
requested changes
Jun 18, 2025
...ed-model/src/funTest/resources/evaluated-model-reporter-test-deduplicate-expected-output.yml
Show resolved
Hide resolved
|
|
||
| /** | ||
| * A map of [Identifier]s associated with a list of [Issue]s that occurred during a scan besides the issues | ||
| * A map of [Identifier]s associated with a set of [Issue]s that occurred during a scan besides the issues |
Member
There was a problem hiding this comment.
I'm not sure if this should already close #10054 because ORT does still provide no way to delete broken scan results from the storage that were created before this change. @sschuberth What do you think?
Member
There was a problem hiding this comment.
You have a point there, but I guess the best we could do is to provide users with a tool / helper-cli command to remove those bogus stored scan results with issues that contain a certain kind of message. As we'd never know if and when users would run the tool, I'd also be fine with closing the issue now, as the root cause is fixed, and provide the mentioned tooling as a follow-up.
Contributor
Author
There was a problem hiding this comment.
The issue also has the following problem described, which this PR doesn't address actually, so I wasn't completely sure otherwise either if this should close the issue:
it should report the error message it gets from ScanCode instead of ignoring the error and throwing a generic FileNotFoundException when it does not find the report
Member
There was a problem hiding this comment.
I'll leave the decision to you two, I'm fine with either outcome.
Add a map for gathering issues that occurred during a scan but which aren't a part of the scan summaries. This will be used to collect issues from failing scans to prevent having to create and save empty scan results which can then cause problems when reading and reusing existing scan results. Signed-off-by: Johanna Lamppu <[email protected]>
As there is now a way to add scan issues that are not tied to scan results, add these issues separately to the `EvaluatedModel` in the `EvaluateModelMapper`. Signed-off-by: Johanna Lamppu <[email protected]>
To resolve an issue where scan results from failed scans were reused even if the problem was resolved in newer scans, change the behavior of the path scanners so that they don't create empty scan results just to keep track of issues, and instead collect these kinds of issues to the `ScannerRun` issues map. Resolves oss-review-toolkit#10054. Signed-off-by: Johanna Lamppu <[email protected]>
lamppu
force-pushed
the
10054-fix-failed-scan-results-reused
branch
from
2d40d5b to
fd11299
Compare
mnonnenmacher
approved these changes
Jun 18, 2025
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 23, 2025
This is required to support the scanenr issues from an ORT `ScannerRun` introduced in [1]. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 24, 2025
[1] added scanner issues to the `ScannerRun`. Add support for this in the server by persisting the issues in the respective database table. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 24, 2025
[1] added scanner issues to the `ScannerRun`. Add support for this in the server by persisting the issues in the respective database table. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 24, 2025
[1] added scanner issues to the `ScannerRun`. Add support for this in the server by persisting the issues in the respective database table. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 24, 2025
[1] added scanner issues to the `ScannerRun`. Add support for this in the server by persisting the issues in the respective database table. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 24, 2025
[1] added scanner issues to the `ScannerRun`. Add support for this in the server by persisting the issues in the respective database table. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 25, 2025
[1] added scanner issues to the `ScannerRun`. Add support for this in the server by persisting the issues in the respective database table. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 26, 2025
[1] added scanner issues to the `ScannerRun`. Add support for this in the server by persisting the issues in the respective database table. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 26, 2025
Persist the issues from `ScannerRun` to the database. The logic for populating these issues in `ScannerRun` was introduced in [1]. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
added a commit
to boschglobal/ort-server
that referenced
this pull request
Jun 26, 2025
Persist the issues from `ScannerRun` to the database. The logic for populating these issues in `ScannerRun` was introduced in [1]. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
github-merge-queue bot
pushed a commit
to eclipse-apoapsis/ort-server
that referenced
this pull request
Jun 26, 2025
Persist the issues from `ScannerRun` to the database. The logic for populating these issues in `ScannerRun` was introduced in [1]. [1]: oss-review-toolkit/ort#10475 Signed-off-by: Marcel Bochtler <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new
issuesproperty to theScannerRunclass and collect issues from completely failing scans for path scanners in the issues map instead of creating and storing empty scan results.Please see the individual commits for details.