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

Resolve plugin config vs classpath exceptions #7723

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

sschuberth
Copy link
Member

Please have a look at the individual commit messages for the details.

Previously, if a provider plugin was enabled in ORT's configuration but
was unavailable in the classpath, the `ALL` map lookup threw a
`NoSuchElementException` resulting in ORT to crash. Avoid that by
logging this case as an error instead, but continuing to run. The ORT
result file will properly reflect only the used provider plugins.

Signed-off-by: Sebastian Schuberth <[email protected]>
In exchange, use the longer but more readable `factory` lambda parameter
name.

Signed-off-by: Sebastian Schuberth <[email protected]>
@sschuberth sschuberth requested a review from a team as a code owner October 20, 2023 07:06
@sschuberth sschuberth enabled auto-merge (rebase) October 20, 2023 07:07
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cb6a186) 67.81% compared to head (c09a355) 67.81%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #7723   +/-   ##
=========================================
  Coverage     67.81%   67.81%           
  Complexity     2045     2045           
=========================================
  Files           352      352           
  Lines         16829    16829           
  Branches       2380     2380           
=========================================
  Hits          11412    11412           
  Misses         4427     4427           
  Partials        990      990           
Flag Coverage Δ
funTest-non-docker 36.44% <100.00%> (ø)
test 34.31% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
.../src/main/kotlin/storages/ClearlyDefinedStorage.kt 83.11% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@oheger-bosch oheger-bosch left a comment

Choose a reason for hiding this comment

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

The change looks good to me. Just wondering whether other plugin types could be affected as well. The commit message is rather generic about "provider plugins".

@sschuberth
Copy link
Member Author

The change looks good to me. Just wondering whether other plugin types could be affected as well. The commit message is rather generic about "provider plugins".

AFAICT, other plugins do not get implicitly created by coming across their configuration, but by filtering enabled ones from those in the classpath.

@sschuberth sschuberth merged commit 94f5687 into main Oct 20, 2023
22 checks passed
@sschuberth sschuberth deleted the plugin-config-vs-classpath branch October 20, 2023 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants