feat: add dryrun codepath for post computes #423
Conversation
|
Relevant connect PR: permaweb/ao#1238 |
jfrain99
changed the title
jfrain99
force-pushed
the
feat/dryrun-genesis-wasm
branch
from
3f0630d to
27c500e
Compare
jfrain99
changed the title
| RawProcessID = dev_process:process_id(Msg1, #{}, Opts), | ||
| case RawProcessID of | ||
| not_found -> hb_ao:get(<<"process-id">>, Msg2, Opts); | ||
| ProcID -> ProcID | ||
| end. |
There was a problem hiding this comment.
This is about idioms but the maybe expression, which is enabled by default in OTP 27+, is very handy for handling a single case:
maybe not_found ?= dev_process:process_id(Msg1, #{}, Opts),
hb_ao:get(<<"process-id">>, Msg2, Opts)
end.
There was a problem hiding this comment.
This will take a transition on erlang idiomatic expressions but the team can have multiple benefits while adopting it. Analysing for this case (and we can extrapolate to many other ones), it helps making clear right away somes points:
- prevents giving two names for the same thing:
RawProcessIDandProcID - that
hb_ao:getdoes not use the output ofdev_process:process_id - subsequent function calls will be made only if all previous conditions are met. This is especially helpful for multiple nested handling.
- as a consequence of the previous point, engineers interested only the case when the
process_idis found don't need to look at any other line of code.
There was a problem hiding this comment.
@twilson63 This might be out of scope for now, but it would be great if we could discuss adopting (or not) this idiom with the team and Sam at a later time, given the benefits mentioned above.
There was a problem hiding this comment.
Yep, we use maybe in other places but I think we format the syntax differently - but maybe is something we have been using so if it fits here, that would be a good change, @jfrain99 I would look at other code examples of maybe to get the formatting correct.
There was a problem hiding this comment.
I could not find any other cases of maybe being used in the codebase - since we are trying to get this across the line, I think now is not the best time to introduce it. It definitely seems like something that should be worked in in the future, though. Thanks!
| extract_json_res(Response, Opts) -> | ||
| case Response of | ||
| {ok, Res} -> | ||
| JSONRes = hb_ao:get(<<"body">>, Res, Opts), | ||
| ?event({ | ||
| delegated_compute_res_metadata, | ||
| {req, hb_maps:without([<<"body">>], Response, Opts)} | ||
| {req, hb_maps:without([<<"body">>], Res, Opts)} |
There was a problem hiding this comment.
Sorry this is a nitpick Jack. I love the way you've refactored it, extracting the code into functions.
I would only keep the previous names/terminology (Res for Results and Response for the second element of the ok tuple).
There was a problem hiding this comment.
Sorry, I am not sure I understand - which variable should be Res / Response? The second element of the ok tuple?
There was a problem hiding this comment.
Res (for Result) having the whole tuple and Response for the element of the tuple 🙏
| <<"hash-chain">> => hb_util:id(HashChain), | ||
| <<"body">> => OnlyAttested | ||
| <<"body">> => OnlyAttested, | ||
| <<"type">> => <<"assignment">> |
There was a problem hiding this comment.
Got it, nice tagging! What about moving <<"type">> to a maps:put inside the commit_assignment so that all commits (this and future ones) of this kind receive this type attribute?
Nvm, all these fields are here to define the assignment, to apply the logic above more fields would need to the moved.
|
@jfrain99 would you mind adding to the description of the PR, the steps that I can follow to test it with a |
|
@jfrain99 ran tests and all appear to be passing |
jfrain99
force-pushed
the
feat/dryrun-genesis-wasm
branch
from
5398394 to
11dfdae
Compare
Rather than determining whether a compute request is a genesis_wasm result/dryrun based on the method, now test if their is an assignment
This reverts commit ba24085.
jfrain99
force-pushed
the
feat/dryrun-genesis-wasm
branch
from
11dfdae to
8a431cf
Compare
Allows for POST /compute call which calls dryrun in the genesis wasm cu.
To simulate a legacy net dryrun, POST compute without an assignment. This can be done using the
as=executionkey. Example call:/<process-id>[email protected]/as=execution/compute<tags>/[email protected], where for example with Action: Info tags areaction=info