upgrade GitHub to use newest HTTP #102
Conversation
src/activity/events.jl
Outdated
| @@ -98,17 +98,17 @@ function handle_event_request(request, handle; | |||
| secret = nothing, events = nothing, | |||
| repos = nothing, forwards = nothing) | |||
| if !(isa(secret, Void)) && !(has_valid_secret(request, secret)) | |||
| return HTTP.Response(400, "invalid signature") | |||
There was a problem hiding this comment.
Not sure how to add these messages.
There was a problem hiding this comment.
@quinnj, @KristofferC, looking at this now...
There was a problem hiding this comment.
See JuliaWeb/HTTP.jl@2c14d43
-- " add constructors: Response(s::Int, body::AbstractString) ..."
Also, maybe this catch is not required because of the catch in HTTP.Servers.handle_stream()
server = HTTP.Servers.Server() do request, response
try
handle_event_request(request, handle; auth = auth,
secret = secret, events = events,
vvvvv repos = repos, forwards = forwards)
catch err
bt = catch_backtrace()
print(STDERR, "SERVER ERROR: ")
Base.showerror(STDERR, err, bt)
return HTTP.Response(500)
end
end
…(s::Int, body::AbstractVector{UInt8}) per JuliaWeb/GitHub.jl#102
Use HTTP.listen instead of HTTP.Servers.serve
src/utils/requests.jl
Outdated
| @@ -146,7 +149,8 @@ function handle_response_error(r::HTTP.Response) | |||
| if r.status >= 400 | |||
| message, docs_url, errors = "", "", "" | |||
| try | |||
| data = JSON.parse(String(r)) | |||
| @show r | |||
|
Should be good to go I think |
|
Pushed some small fixups. Should be good when CI passes. |
|
Looks like you need a dependency on Nullables in order to work on 0.7. |
|
Ok, passes on 0.7 now. Just want to state for the record that test time on 0.7 is significantly worse than on 0.6. Locally, timing 0.6:0.7:Update: Ok, the slowdown is due to deprecations in JSON. Nothing to worry about then. |
| HttpCommon # for deprecations | ||
| HTTP 0.6 | ||
| Nullables | ||
|
|
There was a problem hiding this comment.
There should also be an explicit Compat dependency here, since you're using Nothing and the pairs method for replace (among other things).
There was a problem hiding this comment.
With my test script in Pkg3 this would have errored locally...
So we have it ready for when HTTP tags new release. @Keno, @ararslan
Need to test FemtoCleaner server with this as wellDone