feat: allow address targeting #307
Conversation
walkah
force-pushed
the
walkah/feat-target-address
branch
from
f28710d to
152b8b3
Compare
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @walkah on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again. |
There was a problem hiding this comment.
Looks good! Tested it out and it works happy path and no such resource provider. 👌
Left some comments, minor stuff. One other comment the error case dumps quite a lot of info, including the CLI flags for the command. Would be nice if we could suppress that, but no need for it to block merging this work.
| newTargetOptions, err := ProcessTargetOptions(options.Offer.Target) | ||
| if err != nil { | ||
| return options, err | ||
| } | ||
| options.Offer.Target = newTargetOptions | ||
|
|
There was a problem hiding this comment.
We may be able to skip this processing step because we aren't processing the target. Optional though, alright with adding for the future.
| return resourceOffer, nil | ||
| } | ||
| } | ||
| return nil, nil |
There was a problem hiding this comment.
Could we return an error here? We observed some issues handling errors at the call site:
Lines 150 to 153 in c419ac1
Perhaps we could print the error in this block and continue.
Summary
This adds a new CLI flag
--targetthat allows sending a job to a specific RP address. The solver is updated to look for a target config and skip the regular matching flow to create a deal with the specified RP (if it exists).Task/Issue reference
Closes: #198
Test plan
./lilypad run --network dev --target 0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65 cowsay:v0.0.4 -i Message="hello"--targetshould fail.