🐛 A bug was introduced where we no longer used the opensource labels index #966
Conversation
* There are two index's one for the maven sha search and one for the opensource labels. * The maven SHA search is used by bldtools/decompiler to get the artifact information * the Opensource labels is used by the java-analzyer-bundle to filter out scopes. Signed-off-by: Shawn Hurley <[email protected]>
WalkthroughThe pull request splits a single Maven index path variable into two distinct configuration keys: one for SHA search index and another for open source dependency list, with corresponding updates to initialization and downstream service client construction. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
shawn-hurley
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
external-providers/java-external-provider/pkg/java_external_provider/provider.go(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-30T12:11:45.673Z
Learnt from: pranavgaikwad
Repo: konveyor/analyzer-lsp PR: 859
File: external-providers/java-external-provider/pkg/java_external_provider/dependency.go:694-694
Timestamp: 2025-07-30T12:11:45.673Z
Learning: In the Java external provider dependency walker (external-providers/java-external-provider/pkg/java_external_provider/dependency.go), errors from toDependency function calls should be ignored as they are not considered important by the maintainers.
Applied to files:
external-providers/java-external-provider/pkg/java_external_provider/provider.go
📚 Learning: 2025-08-21T12:39:46.778Z
Learnt from: jmle
Repo: konveyor/analyzer-lsp PR: 888
File: external-providers/java-external-provider/pkg/java_external_provider/provider.go:662-663
Timestamp: 2025-08-21T12:39:46.778Z
Learning: In the konveyor/analyzer-lsp Java provider, when setting JAVA_HOME for Gradle commands using exec.CommandContext, the maintainers prefer not to preserve the existing environment variables (including PATH) and use `cmd.Env = append(cmd.Env, fmt.Sprintf("JAVA_HOME=%s", javaHome))` instead of `cmd.Env = append(os.Environ(), fmt.Sprintf("JAVA_HOME=%s", javaHome))`.
Applied to files:
external-providers/java-external-provider/pkg/java_external_provider/provider.go
🧬 Code graph analysis (1)
external-providers/java-external-provider/pkg/java_external_provider/provider.go (1)
external-providers/java-external-provider/pkg/java_external_provider/dependency/labels/labels.go (1)
ProviderSpecificConfigOpenSourceDepListKey(18-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: test
- GitHub Check: test (macos-latest, darwin, arm64)
- GitHub Check: test (macos-latest, darwin, amd64)
- GitHub Check: test (ubuntu-latest, linux, amd64)
- GitHub Check: test (windows-latest, windows, amd64)
- GitHub Check: test (ubuntu-latest, linux, arm64)
- GitHub Check: benchmark (ubuntu-latest, linux)
- GitHub Check: benchmark (windows-latest, windows)
- GitHub Check: benchmark (macos-latest, mac)
🔇 Additional comments (4)
external-providers/java-external-provider/pkg/java_external_provider/provider.go (4)
281-284: LGTM!The retrieval of
mavenSHASearchIndexproperly handles the case when the config key is not found, with appropriate logging for debugging purposes.
313-313: LGTM!Correctly passing the SHA search index to the build tool options, as intended per the PR description.
345-345: LGTM!Correctly passing the open source index to the service client for filtering scopes, as intended per the PR description.
485-485: LGTM!Correctly passing the open source index to the service client, consistent with the RPC-based initialization on line 345.
| log.Info("unable to find the maven index path in the provider specific config") | ||
| } | ||
|
|
||
| mavenOpenSourceIndex, ok := config.ProviderSpecificConfig[labels.ProviderSpecificConfigOpenSourceDepListKey].(string) |
There was a problem hiding this comment.
Add error handling for missing config key.
The retrieval of mavenOpenSourceIndex is missing the ok check and logging that was added for mavenSHASearchIndex on lines 281-284. This inconsistency could lead to silently using an empty string if the config key is not found.
Apply this diff to add consistent error handling:
mavenOpenSourceIndex, ok := config.ProviderSpecificConfig[labels.ProviderSpecificConfigOpenSourceDepListKey].(string)
+ if !ok {
+ log.Info("unable to find the maven open source index path in the provider specific config")
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mavenOpenSourceIndex, ok := config.ProviderSpecificConfig[labels.ProviderSpecificConfigOpenSourceDepListKey].(string) | |
| mavenOpenSourceIndex, ok := config.ProviderSpecificConfig[labels.ProviderSpecificConfigOpenSourceDepListKey].(string) | |
| if !ok { | |
| log.Info("unable to find the maven open source index path in the provider specific config") | |
| } |
🤖 Prompt for AI Agents
In
external-providers/java-external-provider/pkg/java_external_provider/provider.go
around line 286, the code reads mavenOpenSourceIndex, ok :=
config.ProviderSpecificConfig[labels.ProviderSpecificConfigOpenSourceDepListKey].(string)
but does not handle the case when ok is false; add the same nil/ok check and
logging that exists for mavenSHASearchIndex (lines ~281-284): if the key is
missing or not a string, log a descriptive error (using the same
providerLogger/Errorf or Warnf used earlier) including the config key name and
current ProviderSpecificConfig contents, and set mavenOpenSourceIndex to an
empty string or handle it consistently with the existing behavior used for
mavenSHASearchIndex.
2 participants
Summary by CodeRabbit