fix: return error for unknown resource kind in wait command#384
fix: return error for unknown resource kind in wait command#384shri3016 wants to merge 2 commits intodrasi-project:mainfrom
Conversation
Signed-off-by: shri3016] <shriyaktarcar@gmail.com>
shri3016
force-pushed
the
fix/wait-unknown-resource-type
branch
from
269bc28 to
da13e36
Compare
|
Hello @agentofreality @amansinghoriginal |
amansinghoriginal
left a comment
amansinghoriginal
left a comment
There was a problem hiding this comment.
Thank you for submitting this pull request. We truly appreciate your contributions to Drasi.
To fully fix the reported bug, we need to ensure that the CLI validation catches this "invalid argument count" scenario before passing an empty list to the SDK.
Keep your current changes! They are a valid improvement for handling "Invalid Kind" scenarios (e.g., drasi wait invalidKind myName).
You might need to modify cli/cmd/utils.go to return an error if the argument count is incorrect (i.e., not 0 and not 2).
Let me know if you have any questions as you dig into the code. You're doing great! ✨
Also, engage with our community on our discord server
There was a problem hiding this comment.
Your changes to cli/sdk/api_client.go are solid.
- The logic to check if mf.Kind exists in kindRoutes is correct and idiomatic Go.
- The error message you added is very helpful for users because it explicitly lists the supported resource kinds.
- This improves the robustness of the SDK by preventing invalid requests from being sent to the API.
There was a problem hiding this comment.
@shri3016 Have you had a chance to run the reproduction step (drasi wait aman) locally with your changes applied?
I suspect you might notice that the CLI still appears unresponsive or exits without printing your new error message.
There was a problem hiding this comment.
I've now added argument count validation in cli/cmd/utils.go that returns an error when args is not 0 or 2. After this fix, drasi wait aman correctly shows: Error: invalid argument count: expected 0 or 2 arguments, got 1,Attaching the screenshot below.
| return result, err | ||
| } | ||
|
|
||
| func (t *ApiClient) ReadyWait(manifests *[]api.Manifest, timeout int32, output output.TaskOutput) error { |
There was a problem hiding this comment.
For your validation to run, manifests must contain at least one item.
There was a problem hiding this comment.
I have added validation in cli/cmd/utils.go to catch invalid argument counts before an empty list reaches ReadyWait. Now the error is returned early in loadManifests.
There was a problem hiding this comment.
I suspect that the issue lies upstream in how the CLI parses arguments before it even calls ReadyWait. Take a look at the loadManifests function in cli/cmd/utils.go (line 46).
@shri3016 As a challenge - Trace what happens in loadManifests when args has a length of 1 (like in drasi wait aman)
- Does it enter the if len(args) == 2 block?
- Does it return an error, or does it return an empty list of manifests?
There was a problem hiding this comment.
Traced it! When len(args) == 1: - It does not enter the if len(args) == 2 block - It was returning an empty list (no error). Fixed by adding an else block that returns an error when args count is not 0 or 2.
Thanks for the guidance @amansinghoriginal . Please let me know if I have to make any changes whenever you have time to review this.
Signed-off-by: shri3016] <shriyaktarcar@gmail.com>
shri3016
force-pushed
the
fix/wait-unknown-resource-type
branch
from
a33418f to
dc79635
Compare
2 participants
Description
This PR adds validation for unknown resource kinds in the drasi wait command. Previously, running drasi wait with an invalid resource type (e.g., drasi wait aman) would cause the CLI to hang indefinitely because the code would create a malformed URL and wait forever for a response. Now, the CLI validates the resource kind before making the HTTP request and returns a clear error message: Error: unknown resource kind 'aman'. Supported kinds are: source, reaction, query, continuousquery, querycontainer, sourceprovider, reactionprovider
Type of change
Fixes: #109