🐛 pass build tool when using pipe mode#961
Merged
shawn-hurley merged 2 commits intokonveyor:mainfrom Nov 11, 2025
Merged
Conversation
Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
WalkthroughDeferred RPC client creation in the Java external provider: Init no longer returns an RPC client early; RPC-based javaServiceClient is constructed after project resolution and build tool detection. The analyzer-lsp dependency was bumped from v0.7.0-alpha to v0.8.1-alpha, and javaServiceClient now carries buildTool, mvnIndexPath, and mvnSettingsFile. Changes
Sequence Diagram(s)sequenceDiagram
participant Provider as JavaProvider.Init
participant Resolver as ProjectResolver/BuildTool
participant RPCConf as RPC Config
participant JavaClient as javaServiceClient
rect rgb(230, 245, 255)
Provider->>Resolver: resolve project & detect buildTool
Resolver-->>Provider: project info, buildTool, mvn paths
end
alt RPC config present
Provider->>RPCConf: check RPC settings
RPCConf-->>Provider: RPC config present
Provider->>JavaClient: construct javaServiceClient(buildTool, mvnIndexPath, mvnSettingsFile, includedPaths, depsLocationCache)
JavaClient-->>Provider: client ready
else No RPC config
Provider->>Provider: continue without RPC client (or error later if required)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
approved these changes
Nov 10, 2025
Contributor
shawn-hurley
left a comment
shawn-hurley
left a comment
There was a problem hiding this comment.
Thank you for fixing my bug
…ection being passed Signed-off-by: Shawn Hurley <shawn@hurley.page>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
external-providers/java-external-provider/pkg/java_external_provider/provider.go (2)
270-270: Handle type assertion failure explicitly.The blank identifier silently ignores type assertion failures. If
lspServerPathis configured with the wrong type, it will be treated as an empty string without any indication of the configuration error.Consider explicitly checking the type assertion result:
-lspServerPath, _ := config.ProviderSpecificConfig[provider.LspServerPathConfigKey].(string) +lspServerPath, ok := config.ProviderSpecificConfig[provider.LspServerPathConfigKey].(string) +if !ok && config.ProviderSpecificConfig[provider.LspServerPathConfigKey] != nil { + log.V(5).Info("lspServerPath config value has incorrect type, expected string") +}
347-349: Clarify error message.The error message states "invalid lspServerPath provided" but when
lspServerPathis an empty string, it may not have been provided at all rather than being invalid.Consider a more accurate message:
-} else if lspServerPath == "" { - return nil, additionalBuiltinConfig, fmt.Errorf("invalid lspServerPath provided, unable to init java provider") -} +} else if lspServerPath == "" { + return nil, additionalBuiltinConfig, fmt.Errorf("lspServerPath not provided and RPC client not available, unable to init java provider") +}
📜 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(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: jmle
Repo: konveyor/analyzer-lsp PR: 888
File: external-providers/java-external-provider/pkg/java_external_provider/service_client.go:507-521
Timestamp: 2025-08-21T12:42:54.499Z
Learning: In the konveyor/analyzer-lsp Java external provider (external-providers/java-external-provider/pkg/java_external_provider/service_client.go), the maintainers prefer to keep the current JAVA_HOME selection logic in GetJavaHomeForGradle method simple, as they operate in a controlled environment where both Java 8 and Java 17+ are guaranteed to be available, with Java 17+ being a system requirement for running the analyzer.
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.
📚 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-10-21T16:27:59.216Z
Learnt from: shawn-hurley
Repo: konveyor/analyzer-lsp PR: 860
File: provider/server.go:251-259
Timestamp: 2025-10-21T16:27:59.216Z
Learning: In provider/server.go, the Init method uses context.Background() for creating the RPC client via GetProviderRPCClient because this RPC client is stored in the InitConfig and used across multiple subsequent requests (Evaluate, GetDependencies, etc.). The RPC client's lifecycle is managed separately through explicit Stop calls, not tied to the Init request's context.
Applied to files:
external-providers/java-external-provider/pkg/java_external_provider/provider.go
📚 Learning: 2025-08-21T12:42:54.499Z
Learnt from: jmle
Repo: konveyor/analyzer-lsp PR: 888
File: external-providers/java-external-provider/pkg/java_external_provider/service_client.go:507-521
Timestamp: 2025-08-21T12:42:54.499Z
Learning: In the konveyor/analyzer-lsp Java external provider (external-providers/java-external-provider/pkg/java_external_provider/service_client.go), the maintainers prefer to keep the current JAVA_HOME selection logic in GetJavaHomeForGradle method simple, as they operate in a controlled environment where both Java 8 and Java 17+ are guaranteed to be available, with Java 17+ being a system requirement for running the analyzer.
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
📚 Learning: 2025-10-21T16:25:40.903Z
Learnt from: shawn-hurley
Repo: konveyor/analyzer-lsp PR: 860
File: lsp/base_service_client/base_service_client.go:212-214
Timestamp: 2025-10-21T16:25:40.903Z
Learning: In `lsp/base_service_client/base_service_client.go`, when an RPC connection is provided via `c.RPC` (not nil), it is treated as already initialized and no handler setup is performed. This is known technical debt that needs to be addressed in the future to ensure LSP notifications like `textDocument/publishDiagnostics` are properly processed.
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 (2)
provider/provider.go (2)
LspServerPathConfigKey(37-37)InitConfig(123-159)provider/lib.go (1)
GetIncludedPathsFromConfig(381-404)
⏰ 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 (ubuntu-latest, linux, amd64)
- GitHub Check: test (macos-latest, darwin, amd64)
- GitHub Check: test (macos-latest, darwin, arm64)
- GitHub Check: test (windows-latest, windows, amd64)
- GitHub Check: test (ubuntu-latest, linux, arm64)
- GitHub Check: benchmark (ubuntu-latest, linux)
- GitHub Check: benchmark (macos-latest, mac)
- GitHub Check: benchmark (windows-latest, windows)
🔇 Additional comments (1)
external-providers/java-external-provider/pkg/java_external_provider/provider.go (1)
336-346: LGTM! Build tool information now passed in pipe mode.The RPC-based service client now correctly includes
buildTool,mvnIndexPath, andmvnSettingsFile, making it consistent with the standard initialization path. This addresses the PR objective of passing build tool information when using pipe mode.
This was referenced Nov 18, 2025
This was referenced Jan 28, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary by CodeRabbit
Chores
Refactor