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

Use github.com/ooni/netx: 2/2 #302

Closed
10 tasks done
bassosimone opened this issue Jan 28, 2020 · 1 comment
Closed
10 tasks done

Use github.com/ooni/netx: 2/2 #302

bassosimone opened this issue Jan 28, 2020 · 1 comment
Assignees
@bassosimone
Labels
effort/M Medium effort enhancement New feature or request ooni/probe-engine Issues related to github.com/ooni/probe-engine priority/high High priority
Milestone
Sprint 8 - Flounder

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Jan 28, 2020 •
edited
Loading

We've done some work in #301 and we need to finish the work by removing the remaining code inside of ./httpx and replacing it with the more-or-less corresponding code in github.com/ooni/netx.

As we have discovered when implementing netx, we can be smarter about retries than just redoing the whole Dial operation. So, it may even be fine to not re-add a retry mechanism.
@bassosimone bassosimone added enhancement New feature or request priority/medium Medium priority effort/M Medium effort ooni/probe-engine Issues related to github.com/ooni/probe-engine labels Jan 28, 2020
@bassosimone bassosimone added this to the Sprint 6 - Dumbo Octopus milestone Jan 28, 2020
@bassosimone bassosimone self-assigned this Jan 28, 2020
@bassosimone bassosimone added priority/high High priority and removed priority/medium Medium priority labels Mar 2, 2020
@bassosimone bassosimone mentioned this issue Mar 3, 2020
Merged
bassosimone added a commit that referenced this issue Mar 3, 2020
@bassosimone
internal: remove the retrying dialer
72f566d 
This is part of #302
@bassosimone bassosimone mentioned this issue Mar 3, 2020
Merged
bassosimone added a commit that referenced this issue Mar 3, 2020
@bassosimone
internal: remove the retrying dialer (#379)
6740c6e 
This is part of #302
bassosimone added a commit that referenced this issue Mar 3, 2020
@bassosimone
Vendor ooni/netx@4f8d645bce and use it
a313be2 
The vendoring has omitted files we don't care about. The source code is
not changed. The README.md and DESIGN.md file have been updated.

Part of #302.
@bassosimone bassosimone mentioned this issue Mar 3, 2020
Merged
bassosimone added a commit that referenced this issue Mar 3, 2020
@bassosimone
Vendor ooni/netx@4f8d645bce and use it (#380)
d9d546e 
The vendoring has omitted files we don't care about. The source code is
not changed. The README.md and DESIGN.md file have been updated.

Part of #302.
This was referenced Mar 3, 2020
Closed
Merged
bassosimone added a commit that referenced this issue Mar 3, 2020
@bassosimone
netx: unify netx and httpx packages (#381)
54fd287 
Part of #302
bassosimone added a commit that referenced this issue Mar 4, 2020
@bassosimone
netx: merge public API with internal
f7b08ca 
Closes #338, because I have
added code and unit test to check for such case.

Closes #310.

Part of #302.
@bassosimone bassosimone mentioned this issue Mar 4, 2020
Merged
bassosimone added a commit that referenced this issue Mar 4, 2020
@bassosimone
netx: merge public API with internal (#382)
658a781 
Closes #338, because I have
added code and unit test to check for such case.

Closes #310.

Part of #302.
bassosimone added a commit that referenced this issue Mar 4, 2020
@bassosimone
Replace internal/httpx code with netx
2fe4aea 
This is part of #302
@bassosimone bassosimone mentioned this issue Mar 4, 2020
Merged
bassosimone added a commit that referenced this issue Mar 5, 2020
@bassosimone
Replace internal/httpx code with netx (#383)
9f9027b 
This is part of #302.

I feel comfortable with removing the `--ca-bundle-path <path>` command line option from `miniooni` because (1) we don't provide CLI stability guarantees and (2) we were aiming to provide a CLI that was compatible with OONI Probe v2.x CLI, which had no such option.
bassosimone added a commit that referenced this issue Mar 5, 2020
@bassosimone
ExperimentMeasurer describes how an experiment measures
f1c092b 
This diff modifies the way in which we tell the generic experiment
code about the specific experiment code.

This change is motived by these three issues:

1. #184

2. #183

3. #302

Very briefly, I have discovered that I need #184 and #185 in
order to do #302 in a clean way. In fact, to properly call #302
finished, I need to add to the session a Close method. This
method will guarantee that we don't leak open TCP connections
into the HTTP servers managed by a session.

While working on that, I noticed that the technical debt entailed
by #184 and #185 started to be a bit too much. So, this work
aimed at making sure that we reduce the technical debt.

The general plan is to decouple the implementation of tests from
both the generic experiment and the generic session.

This diff addresses decoupling the specific experiment from the
generic experiment. To this end we introduce this ExperimentMeasurer
interface that now the generic experiment expects.

We cannot put this interface into the model package now, because of
a circular import. However, in the next commit will make it such that
this interface does not depend from session.

Once that is done, we can do #184 and #185 and finally #302.
@bassosimone bassosimone mentioned this issue Mar 6, 2020
Closed
This was referenced Mar 6, 2020
Closed
Merged
bassosimone added a commit that referenced this issue Mar 6, 2020
@bassosimone
[API CHANGE] remove session/session.go and experiment/experiment.go (#…
4426ef2 
…385)

As mentioned in #302, this split of the experiment and session was complicating applying further changes. It looked like the right moment to remove this source of technical debt and rationalise the implementation. Also, it's worth noting that we now have a strategy for producing mobile bindings that does not use the top-level "engine" API. So, a bunch of complexities that have been introduced, can be removed, making the "engine" API more idiomatic.

Summary of API changes:

1. log.Logger => model.Logger

2. ExperimentBuilder.{Build => NewExperiment}

3. engine.Callbacks => model.ExperimentCallbacks

4. added model.ExperimentMeasurer: this is the API between generic and specific experiment code

5. engine.Measurement merged with model.Measurement

6. added model.ExperimentSession: API exposed by session to specific experiment code

7. added model.ExperimentOrchestraClient: orchestra client exposed to specific experiment code

8. note that the testlists API may be deprecated

Closes #183

Closes #184
bassosimone added a commit that referenced this issue Mar 6, 2020
@bassosimone
session.go: ensure we close idle connections
247a484 
Part of #302
@bassosimone bassosimone mentioned this issue Mar 6, 2020
Merged
bassosimone added a commit that referenced this issue Mar 6, 2020
@bassosimone
session.go: ensure we close idle connections (#386)
d50ef4f 
Part of #302

This is marked API CHANGE because now consumers must call sess.Close.
bassosimone added a commit that referenced this issue Mar 6, 2020
@bassosimone
testlists.go: use naming consistent with other files
3e00683 
Makes the documentation more beautiful.

Part of #302.
@bassosimone bassosimone mentioned this issue Mar 6, 2020
Merged
bassosimone added a commit that referenced this issue Mar 6, 2020
@bassosimone
testlists.go: use naming consistent with other files (#387)
8b8c2ef 
Makes the documentation more beautiful.

Part of #302.
@bassosimone
Copy link
Contributor Author

All done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
effort/M Medium effort enhancement New feature or request ooni/probe-engine Issues related to github.com/ooni/probe-engine priority/high High priority
Projects
None yet
Milestone
Sprint 8 - Flounder
Development

No branches or pull requests
1 participant
@bassosimone