prototype(probeservices): annotate request and response with their type #997
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This diff changes httpapi Endpoint and Descriptor such that the logger field is part of the Endpoint rather than Descriptor. This changes makes sense because the logger to use should be the same for every API call. What matters, instead, is whether to log bodies, which remains a property of the API Descriptor.
The current httpapi code has the concept of Descriptor, which describes how an API call should be made in terms of its request, but which lacks a precise response definition. Because our goal is to automatically generate swaggers from the API spec, we need a more abstract way of describing an API. This way will be the Spec. We will have two kind of Spec: 1. `SimpleSpec` is the specification for an API that returns a raw response only consisting of bytes; 2. `TypedSpec[T]` is the specification for an API that returns `*T` responses on success. From a Spec, you can generate the `Descriptor`, so the previous code does not change much. Also, while there, recognize that, with a little tweak, we don't need the SequenceCaller but we can do all the work inside of the new APIs for calling specs. Additionally, deprecate previous code that is now superseded by this new approach--without removing this code for now.
The SequenceCaller concept is useful because potentially it can incapsulate some state. Thus, in the future we may be able to try the list of endpoints in the correct order. So, it's actually worth keeping this concept and so we need two distinct callers one for each possible calling style. (I am starting to wonder whether this is a place where we could use monads to decorate return values?)
bassosimone
force-pushed
the
issues/2372
branch
from
aa60204 to
24d9c22
Compare
It _seems_ I should be able to take advantage of the new style to automatically generate a swagger to validate against ooni/api's one.
bassosimone
added a commit
that referenced
this pull request
Dec 9, 2022
This diff changes httpapi Endpoint and Descriptor such that the logger field is part of the Endpoint rather than Descriptor. This changes makes sense because the logger to use should be the same for every API call. What matters, instead, is whether to log bodies, which remains a property of the API Descriptor. While I authored this diff originally as part of the work done in #997, it seems it's required in #1002 as well. Hence, the reference issue is ooni/probe#2379.
4 tasks
bassosimone
added a commit
that referenced
this pull request
Dec 9, 2022
This diff changes httpapi Endpoint and Descriptor such that the logger field is part of the Endpoint rather than Descriptor. This changes makes sense because the logger to use should be the same for every API call. What matters, instead, is whether to log bodies, which remains a property of the API Descriptor. While I authored this diff originally as part of the work done in #997, it seems it's required in #1002 as well. Hence, the reference issue is ooni/probe#2379. While there, fix the go1.19 build workflow, which was broken by using actions/setup-go rather than downloading go manually.
bassosimone
added a commit
that referenced
this pull request
Dec 14, 2022
Originally included in #997. Reference issue: ooni/probe#2380.
Conflicts: internal/engine/probeservices/checkreportid.go internal/httpapi/descriptor.go
|
Reference issue was ooni/probe#2372. |
bassosimone
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Checklist
ooni/apiprobe#2372Description
This pull request contains an early prototype of annotating the (JSON) request and response of probeservices with their type such that we can generate a swagger to compare to the API's swagger and implement ooni/probe#2372.