Skip to content
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.

Sign up for GitHub

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

Jump to bottom

probeservices: sync up data formats with ooni/api #2372

Closed
Tracked by #2362
bassosimone opened this issue Nov 30, 2022 · 0 comments
Closed
Tracked by #2362

probeservices: sync up data formats with ooni/api #2372

bassosimone opened this issue Nov 30, 2022 · 0 comments
Assignees
@bassosimone
Labels
chore routine tasks that must be done, but require little active brain power funder/drl2020-2022 ooni/probe-engine priority/high

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Nov 30, 2022 •
edited
Loading

In particular, we need to take care of the check-in data format.

Part of #2362.
@bassosimone bassosimone mentioned this issue Nov 30, 2022
Closed
9 tasks
@bassosimone bassosimone self-assigned this Nov 30, 2022
@bassosimone bassosimone added priority/high chore routine tasks that must be done, but require little active brain power ooni/probe-engine funder/drl2020-2022 labels Nov 30, 2022
@bassosimone bassosimone changed the title sync up data formats with the ooni/api (in particular the check-in data format) probeservices: sync up data formats with ooni/api Nov 30, 2022
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
refactor(model): improve ooapi.go documentation
e72ad1f 
Part of ooni/probe#2372
@bassosimone bassosimone mentioned this issue Nov 30, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
refactor(model): improve ooapi.go documentation (#992)
69983fc 
Part of ooni/probe#2372
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
refactor(testorchestra): merge with probeservices
a3192fe 
This cleanup is part of ooni/probe#2372
@bassosimone bassosimone mentioned this issue Nov 30, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
refactor(testorchestra): merge with probeservices (#993)
932a26a 
This cleanup is part of ooni/probe#2372
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
refactor(probeservices): use internal testing
427c064 
I was trying to (1) either move data types definitions inside probeservices
to internal/model or (2) make them private to reduce the amount of
exported definitions. However, I was severely limited in doing that
by the fact that probeservices uses external testing.

To overcome this issue, I'm switching to internal testing first and then
I will resume with the two points above, which are in line with my goal
for today of moving ooni/probe#2372 forward.
@bassosimone bassosimone mentioned this issue Nov 30, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
refactor(probeservices): use internal testing (#994)
f8edd33 
I was trying to (1) either move data types definitions inside probeservices
to internal/model or (2) make them private to reduce the amount of
exported definitions. However, I was severely limited in doing that
by the fact that probeservices uses external testing.

To overcome this issue, I'm switching to internal testing first and then
I will resume with the two points above, which are in line with my goal
for today of moving ooni/probe#2372 forward.
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
refactor(probeservices): move data model inside internal/model
f004d80 
This diff refactors probeservices the move the full data model inside
of internal/model. Once I have all the model in there, I can re-introduce
code we sketched out in the past to keep the model in sync with the
API's swagger. Doing that without all the sources in the same directory
would have been quite a bit more difficult to do.

Part of ooni/probe#2372
@bassosimone bassosimone mentioned this issue Nov 30, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
refactor(probeservices): move data model inside internal/model (#995)
98715c5 
This diff refactors probeservices the move the full data model inside
of internal/model. Once I have all the model in there, I can re-introduce
code we sketched out in the past to keep the model in sync with the
API's swagger. Doing that without all the sources in the same directory
would have been quite a bit more difficult to do.

Part of ooni/probe#2372
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
probeservices: sync up with v3.14.0 ooapi package
136679e 
This diff ensures that the definitions we're using are in sync with
the model used by [v3.14.0's ooapi package](
https://github.com/ooni/probe-cli/tree/v3.14.0/internal/ooapi/apimodel),
which was an experiment at autogenerating APIs and cross checking
with ooni/api's swagger JSON file.

The only difference that remains pertains the name of the query
string parameter for /api/v1/test-list/urls, which must be plural
(as it is in the probeservices package) rather than singular (as
it is in the ooapi/apimodel package).

So, after merging this diff, we can say we have done part of the
work specified by ooni/probe#2372. However,
we want to also check with the _current_ swagger.

(Because we already had a model that was consistent with the
swagger _at the time we wrote ooapi_, it seems reasonable for
us to start syncing up with that model, which is easy, and
then focus on importing the swagger and comparing with it.)
@bassosimone bassosimone mentioned this issue Nov 30, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Nov 30, 2022
@bassosimone
probeservices: sync up with v3.14.0 ooapi package (#996)
535c912 
This diff ensures that the definitions we're using are in sync with
the model used by [v3.14.0's ooapi package](
https://github.com/ooni/probe-cli/tree/v3.14.0/internal/ooapi/apimodel),
which was an experiment at autogenerating APIs and cross checking
with ooni/api's swagger JSON file.

The only difference that remains pertains the name of the query
string parameter for /api/v1/test-list/urls, which must be plural
(as it is in the probeservices package) rather than singular (as
it is in the ooapi/apimodel package).

So, after merging this diff, we can say we have done part of the
work specified by ooni/probe#2372. However,
we want to also check with the _current_ swagger.

(Because we already had a model that was consistent with the
swagger _at the time we wrote ooapi_, it seems reasonable for
us to start syncing up with that model, which is easy, and
then focus on importing the swagger and comparing with it.)
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 14, 2022
@bassosimone
cleanup(httpapi): remove optional logging-related complexity
c558a14 
1. we don't care _this much_ about body logging that we need to have
a specific test making sure we _actually_ log the bodies. If we break
body logging by incident, that's annoying but not really bad. In
exchange, we remove the most complex test of this package.

2. it's pointless to log that the content-type header is not the
expected one when expecting JSON. In case of parsing failure, the
user could use `-v` to turn on verbose mode and see logs. Hence,
we don't need to be this picky and this also allows us to zap some
extra code and testing complexity.

3. while there, avoid using `|foo|` in documentation comments
because the proper form is actually `[foo]`.

All this work is really just yak shaving to reduce the noise in a
diff for ooni/probe#2372.
@bassosimone bassosimone mentioned this issue Dec 14, 2022
Merged
4 tasks
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 14, 2022
@bassosimone
cleanup(httpapi): remove optional logging-related complexity (#1008)
4f4e56b 
1. we don't care _this much_ about body logging that we need to have
a specific test making sure we _actually_ log the bodies. If we break
body logging by incident, that's annoying but not really bad. In
exchange, we remove the most complex test of this package.

2. it's pointless to log that the content-type header is not the
expected one when expecting JSON. In case of parsing failure, the
user could use `-v` to turn on verbose mode and see logs. Hence,
we don't need to be this picky and this also allows us to zap some
extra code and testing complexity.

3. while there, avoid using `|foo|` in documentation comments
because the proper form is actually `[foo]`.

All this work is really just yak shaving to reduce the noise in a
diff for ooni/probe#2372.
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 14, 2022
@bassosimone
feat(httpapi): annotate Descriptor with RequestType
5caf6c8 
This diff changes the httpapi to annotate a Descriptor with the
RequestType. Internally, the descriptor will include a RequestDescriptor
that for now only contains the body. Soon, it may contain more fields.

With the Descriptor annotated with a request type, we're now well
positioned to generate half of the swagger.

The reference issue is ooni/probe#2372
@bassosimone bassosimone mentioned this issue Dec 14, 2022
Merged
4 tasks
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 14, 2022
@bassosimone
feat(httpapi): annotate Descriptor with RequestType (#1010)
7fa53b4 
This diff changes the httpapi to annotate a Descriptor with the
RequestType. Internally, the descriptor will include a RequestDescriptor
that for now only contains the body. Soon, it may contain more fields.

With the Descriptor annotated with a request type, we're now well
positioned to generate half of the swagger.

The reference issue is ooni/probe#2372
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 14, 2022
@bassosimone
feat(httpapi): annotate Descriptor with ResponseType
7f3a4d7 
This diff modifies httpapi to annotate the descriptor with the
ResponseType, which also allows us to write code where we naturally
call an API as follows:

```Go
resp, err := httpapi.Call(ctx, desc, epnt)
```

and where `resp` is of the correct response type and where `desc`
has been configured to unmarshal using the JSON format.

In addition to this syntactic-sugar change, this diff matters because
knowing the type of the request and of the response allows us to
generate a swagger to compare to the server's swagger.

Part of ooni/probe#2372.
@bassosimone bassosimone mentioned this issue Dec 14, 2022
Merged
4 tasks
bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 16, 2022
@bassosimone
feat(httpapi): annotate Descriptor with ResponseType (#1011)
9c7e0c6 
This diff modifies httpapi to annotate the descriptor with the
ResponseType, which also allows us to write code where we naturally
call an API as follows:

```Go
resp, err := httpapi.Call(ctx, desc, epnt)
```

and where `resp` is of the correct response type and where `desc`
has been configured to unmarshal using the JSON format.

In addition to this syntactic-sugar change, this diff matters because
knowing the type of the request and of the response allows us to
generate a swagger to compare to the server's swagger.

Part of ooni/probe#2372.
@bassosimone bassosimone mentioned this issue Dec 16, 2022
Closed
4 tasks
@bassosimone bassosimone mentioned this issue Jan 17, 2023
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 6, 2023
@bassosimone
fix(model): get TH info from check-in API
8b058fc 
Part of ooni/probe#2372
@bassosimone bassosimone mentioned this issue Feb 6, 2023
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Feb 6, 2023
@bassosimone
fix(model): get TH info from check-in API (#1076)
71d40e3 
Part of ooni/probe#2372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
chore routine tasks that must be done, but require little active brain power funder/drl2020-2022 ooni/probe-engine priority/high
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bassosimone @jbonisteel