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

minioonirunv2: minimal oonirun v2 implementation #2184

Closed
bassosimone opened this issue Jul 8, 2022 · 1 comment · Fixed by ooni/probe-cli#916
Closed

minioonirunv2: minimal oonirun v2 implementation #2184

bassosimone opened this issue Jul 8, 2022 · 1 comment · Fixed by ooni/probe-cli#916
Assignees
@bassosimone
Labels
enhancement improving existing code or new feature ooni/probe-engine priority/high research prototype user feedback requests that have been added to the backlog as a direct result of user feedback or testing

Comments

@bassosimone
Copy link
Contributor

This issue is about adding a minimal oonirun v2 implementation to miniooni. We will need to write a short design document explaining how we're going to grow this minimal implementation to a full oonirun v2 implementation.
@bassosimone bassosimone self-assigned this Jul 8, 2022
@bassosimone bassosimone added enhancement improving existing code or new feature priority/high user feedback requests that have been added to the backlog as a direct result of user feedback or testing research prototype ooni/probe-engine and removed triage labels Jul 8, 2022
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
refactor(engine): *http.Client -> model.HTTPClient
af647ab 
This diff makes the implementation of the engine package more
abstract by changing HTTPClient() to return a model.HTTPClient
as opposed to returning an *http.Client.

Part of ooni/probe#2184
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
refactor(engine): *http.Client -> model.HTTPClient (#836)
6019b25 
This diff makes the implementation of the engine package more
abstract by changing HTTPClient() to return a model.HTTPClient
as opposed to returning an *http.Client.

Part of ooni/probe#2184
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
refactor(engine): set options from any value
2bb58e3 
This diff refactors how we set options for experiments to accept
in input an any value or a map[string]any, depending on which method
we choose to actually set options.

There should be no functional change, except that now we're not
guessing the type and then attempting to set the value of the selected
field: now, instead, we match the provided type and the field's type
as part of the same function (i.e., SetOptionAny).

This diff is functional to ooni/probe#2184,
because it will allow us to load options from a map[string]any,
which will be part of the OONI Run v2 JSON descriptor.

If we didn't apply this change, we would only have been to set options
from a map[string]string, which is good enough as a solution for the
CLI but is definitely clumsy when you have to write stuff like:

```JSON
{
  "options": {
    "HTTP3Enabled": "true"
  }
}
```

when you could instead more naturally write:

```JSON
{
  "options": {
    "HTTP3Enabled": true
  }
}
```
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
refactor(engine): set options from any value (#837)
086ae43 
This diff refactors how we set options for experiments to accept
in input an any value or a map[string]any, depending on which method
we choose to actually set options.

There should be no functional change, except that now we're not
guessing the type and then attempting to set the value of the selected
field: now, instead, we match the provided type and the field's type
as part of the same function (i.e., SetOptionAny).

This diff is functional to ooni/probe#2184,
because it will allow us to load options from a map[string]any,
which will be part of the OONI Run v2 JSON descriptor.

If we didn't apply this change, we would only have been to set options
from a map[string]string, which is good enough as a solution for the
CLI but is definitely clumsy when you have to write stuff like:

```JSON
{
  "options": {
    "HTTP3Enabled": "true"
  }
}
```

when you could instead more naturally write:

```JSON
{
  "options": {
    "HTTP3Enabled": true
  }
}
```
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
refactor(engine): more abstract Experiment{,Builder}
4e65995 
This diff modifies the engine package to make Experiment and
ExperimentBuilder interfaces rather than structs.

The previosuly existing structs are now named experiment{,Builder}.

This diff helps ooni/probe#2184
because it allows us to write unit tests more easily.

There should be no functional change.

While there, I removed a bunch of deprecated functions, which were
unnecessarily complicate the implementation and could be easily
replaced by passing them a context.Context or context.Background().
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
refactor(engine): more abstract Experiment{,Builder} (#838)
97864b3 
This diff modifies the engine package to make Experiment and
ExperimentBuilder interfaces rather than structs.

The previosuly existing structs are now named experiment{,Builder}.

This diff helps ooni/probe#2184
because it allows us to write unit tests more easily.

There should be no functional change.

While there, I removed a bunch of deprecated functions, which were
unnecessarily complicate the implementation and could be easily
replaced by passing them a context.Context or context.Background().
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
cleanup(miniooni): remove --limit option
74d19a2 
This option has been disabled for a long time and we said in the
codebase we were going to remove it after 2021-11-01.

So, it feels okay to remove it.

This diff is a cleanup in preparation for ooni/probe#2184.
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
cleanup(miniooni): remove --limit option (#840)
a960ca5 
This option has been disabled for a long time and we said in the
codebase we were going to remove it after 2021-11-01.

So, it feels okay to remove it.

This diff is a cleanup in preparation for ooni/probe#2184.
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
fix(engine): repair broken integration test
f915cb0 
The integration test that was broken was:

```
--- FAIL: TestCreateInvalidExperiment (0.35s)
    experiment_integration_test.go:192: expected a nil builder here
```

While there improve the documentation of the ExperimentSession
and see there's a method that we are not using.

This diff is a cleanup that I come up with while working
on ooni/probe#2184.
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
fix(engine): repair broken integration test (#841)
e5697e6 
The integration test that was broken was:

```
--- FAIL: TestCreateInvalidExperiment (0.35s)
    experiment_integration_test.go:192: expected a nil builder here
```

While there improve the documentation of the ExperimentSession
and see there's a method that we are not using.

This diff is a cleanup that I come up with while working
on ooni/probe#2184.
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
feat: start sketching out the oonirun package
1cf1235 
This diff refactors the ./internal/cmd/miniooni pkg and moves the code
for running experiments inside of the ./internal/oonirun pkg.

It's the first concrete step towards ooni/probe#2184.
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
feat: start sketching out the oonirun package (#842)
0b4a491 
This diff refactors the ./internal/cmd/miniooni pkg and moves the code
for running experiments inside of the ./internal/oonirun pkg.

It's the first concrete step towards ooni/probe#2184.
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
feat(oonirun): implement OONIRun v1
1b63a87 
This diff adds support for running OONIRun v1 links.

Run with `miniooni` using:

```
./miniooni -i LINK oonirun
```

Part of ooni/probe#2184
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
feat(oonirun): implement OONIRun v1 (#843)
ebb78c2 
This diff adds support for running OONIRun v1 links.

Run with `miniooni` using:

```
./miniooni -i LINK oonirun
```

Part of ooni/probe#2184
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
feat(oonirun): add support for OONIRun v2 links
f038153 
This diff adds support for OONIRun v2 links. Documentation regarding
how to use these links will follow soon.

Part of ooni/probe#2184.
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
@bassosimone
Copy link
Contributor Author

bassosimone commented Jul 8, 2022 •
edited
Loading

Here's what remains to do:

When we merge ooni/probe-cli#844, we'll already have a good prototype, though.

bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
feat(oonirun): add support for OONIRun v2 links (#844)
9a0153a 
This diff adds support for OONIRun v2 links.

Part of ooni/probe#2184.
@bassosimone bassosimone mentioned this issue Jul 8, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jul 8, 2022
@bassosimone
feat(miniooni): introduce the --repeat-every command line flag (#819)
8aad36a 
Until OONI Run v2 has support for repeating the measurement with a schedule, introduce a command line flag requested by users to repeat a measurement every given number of seconds.

Part of ooni/probe#2184
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 31, 2022
@bassosimone
refactor(miniooni): divide et impera
9432864 
This diff splits miniooni's implementation in smaller and more
easily tractable blocks ahead of future refactoring.

I'm trying to make `miniooni oonirun -i URL` as possible as
`miniooni -i URL oonirun`, because users typically expect this
kind of flexibity from modern Unix commands.

Part of ooni/probe#2184
@bassosimone bassosimone mentioned this issue Aug 31, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 31, 2022
@bassosimone
refactor(miniooni): divide et impera (#912)
7daa686 
This diff splits miniooni's implementation in smaller and more
easily tractable blocks ahead of future refactoring.

I'm trying to make `miniooni oonirun -i URL` as possible as
`miniooni -i URL oonirun`, because users typically expect this
kind of flexibity from modern Unix commands.

Part of ooni/probe#2184
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 31, 2022
@bassosimone
feat(miniooni): make CLI much more user friendly
f5e149c 
Part of ooni/probe#2184, because I wanted
to allow swapping commands and options more freely.

AFAICT, every usage that was legal before is still legal. What has
changed seems the freedom to swap commands and options and a much
better help that lists the available options.
@bassosimone bassosimone mentioned this issue Aug 31, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 31, 2022
@bassosimone
feat(miniooni): make CLI much more user friendly (#913)
0bc6aae 
Part of ooni/probe#2184, because I wanted
to allow swapping commands and options more freely.

As a side effect, this PR closes ooni/probe#2248.

AFAICT, every usage that was legal before is still legal. What has
changed seems the freedom to swap commands and options and a much
better help that lists the available options.
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 31, 2022
@bassosimone
feat(oonirun): improve tests
e8e61c9 
See ooni/probe#2184
@bassosimone bassosimone mentioned this issue Aug 31, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 31, 2022
@bassosimone
feat(oonirun): improve tests (#915)
d0da224 
See ooni/probe#2184

While there, rename `runtimex.PanicIfFalse` to `runtimex.Assert` (it was about time...)
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 31, 2022
@bassosimone
doc: document the minioonirunv2 experiment
504cadb 
Closes ooni/probe#2184
@bassosimone bassosimone mentioned this issue Aug 31, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 31, 2022
@bassosimone
doc: document the minioonirunv2 functionality (#916)
860426b 
Closes ooni/probe#2184
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
enhancement improving existing code or new feature ooni/probe-engine priority/high research prototype user feedback requests that have been added to the backlog as a direct result of user feedback or testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

doc: document the minioonirunv2 functionality ooni/probe-cli
1 participant
@bassosimone