Fixed main-only bug in multi deployer. #422
Merged
Conversation
This file contains 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
spetrovic77
approved these changes
Jun 28, 2023
Before this PR, if you tried to `weaver multi deploy` a Service Weaver application that only had a main component, it would error out immediately with a "github.com/ServiceWeaver/weaver/Main not found". Here's what was happening. `weaver multi deploy` read the component callgraph from the application binary to know which components existed. Reading the callgraph returns the edges in the graph. However, for an application with only a main component, there are no edges. Thus, `weaver multi deploy` erroneously concluded that there are no components. This PR makes a dirt simple fix. We may want to revise the callgraph API to return nodes and edges in a future PR to make this problem harder to run into? Also, sadly, applications that exit from main still exhibit odd behavior when deployed with `weaver multi deploy`. The hello world application added in this PR for example, doesn't print anything. I believe we aren't waiting to flush all messages over the pipe, but I'll have to debug further in a future PR.
Yeah, me too! |
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Before this PR, if you tried to
weaver multi deploya Service Weaver application that only had a main component, it would error out immediately with a "github.com/ServiceWeaver/weaver/Main not found".Here's what was happening.
weaver multi deployread the component callgraph from the application binary to know which components existed. Reading the callgraph returns the edges in the graph. However, for an application with only a main component, there are no edges. Thus,weaver multi deployerroneously concluded that there are no components.This PR makes a dirt simple fix. We may want to revise the callgraph API to return nodes and edges in a future PR to make this problem harder to run into?
Also, sadly, applications that exit from main still exhibit odd behavior when deployed with
weaver multi deploy. The hello world application added in this PR for example, doesn't print anything. I believe we aren't waiting to flush all messages over the pipe, but I'll have to debug further in a future PR.