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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(lib/runtime): Implement
ext_offchain_http_request_add_header_version_1
host function #1994feat(lib/runtime): Implement
ext_offchain_http_request_add_header_version_1
host function #1994Changes from 40 commits
2899a29
8f7a581
4d6167f
faa5752
e08300e
70e6351
ad8c0bb
5606599
f2ca718
81c0fb9
6da5ad8
fb21f11
881fc51
7444590
8896ae5
ee372c4
78bfd0b
42ff50e
13d540a
60a1f11
4cd379f
f1130ad
5f980ab
c8913d2
fcb5da9
9d2b7c5
dd89472
8624e09
427c183
b791f35
750caa9
6cdef2b
c856a87
85c4703
d203860
c8463f8
5353927
281ed22
a7e74d3
18a2cc8
2e57583
d02a434
eee9136
9410059
a4441a2
92b9f49
75af512
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use http.Request's context to store those two booleans? Just to reduce the Go API. Although it might be not worth it depending how many times we check these values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe we can get improvements storing these values on
http.Request's context
the reason we had these values is to avoid operations over already ongoing or finalized HTTP callsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not for improvements except to use
*http.Request
directly. Most http routers store stuff in the http.Request's context, I think it would fit nicely, so we would have one less type to manage ourselves.Example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qdm12, done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this concurrent safe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, when are these variables set and what are they used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now, there is no concurrency (will be implemented here #2020). The
invalid
andwaiting
are set totrue
when we're waiting for a response, then thewaiting
is set tofalse
when we got a successful or failed response howeverinvalid
remainstrue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see where in the code they're set (unless it wasn't in this PR?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, right now these values aren't being set because the function that does this is not implemented yet!