refactor: introduce and use InputOrStaticDefault #632
Merged
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 commit introduces a new `InputLoader` policy by which, if no input is provided, we use a static default input list. We also modify the code to use this policy for dnscheck and stunreachability, with proper input. We also modify `miniooni` to pass the new `ExperimentName` field to the `InputLoader` to indicate which default input list to use. This diff is part of a set of diffs aiming at fixing ooni/probe#1814 and has been extracted from #539. What remains to be done, after this diff has landed is to ensure things also work for ooniprobe and oonimkall.
5 tasks
bassosimone
added a commit
that referenced
this pull request
Dec 3, 2021
This commit introduces a new `InputLoader` policy by which, if no input is provided, we use a static default input list. We also modify the code to use this policy for dnscheck and stunreachability, with proper input. We also modify `miniooni` to pass the new `ExperimentName` field to the `InputLoader` to indicate which default input list to use. This diff is part of a set of diffs aiming at fixing ooni/probe#1814 and has been extracted from #539. What remains to be done, after this diff has landed is to ensure things also work for ooniprobe and oonimkall.
bassosimone
added a commit
that referenced
this pull request
Dec 3, 2021
* [backport] refactor(stunreachability): input required and must be an URL (#630) Here we're refactoring stunreachability to not provide internally a default input and to take in input an URL rather than a string. The related ooni/spec change is ooni/spec#227. This diff has been extracted from #539. Because the original diff was large, I'm splitting it in a set of more easily manageable diffs. The reference issue is ooni/probe#1814, which is complex enough to require us to proceed incrementally. This diff WILL need to be backported to release/3.11. * [backport] refactor: create common package for holding STUN input (#631) We want stunreachability to use the same STUN servers used by snowflake, so let's start by making a common package holding the servers. Let's also use this new package in Snowflake. We're currently not using this package in stunreachability, but I am going to apply this as a subsequent diff. Reference issue: ooni/probe#1814. This issue is a bit complex to address in a single PR, so we are going to proceed incremntally. This diff was extracted from #539. * [backport] refactor: introduce and use InputOrStaticDefault (#632) This commit introduces a new `InputLoader` policy by which, if no input is provided, we use a static default input list. We also modify the code to use this policy for dnscheck and stunreachability, with proper input. We also modify `miniooni` to pass the new `ExperimentName` field to the `InputLoader` to indicate which default input list to use. This diff is part of a set of diffs aiming at fixing ooni/probe#1814 and has been extracted from #539. What remains to be done, after this diff has landed is to ensure things also work for ooniprobe and oonimkall. * [backport] fix(ooniprobe): dnscheck,stunreachability run w/ default input (#633) This diff is part of ooni/probe#1814 and teaches `ooniprobe` to run dnscheck and stunreachability by using the default static input feature of the `InputLoader`. I've manually tested that we can still run `websites` like we did before (including category filtering). I've also manually tested that now we can run `experimental` and get parseable results for dnscheck and stunreachability. With this diff in, we have fixed the original problem highlighted in the ooni/probe#1814 issue. Yet, because of the way in which I solved the problem, there is more work to do. My changes have broken stunreachability for mobile and now it's time I apply fixes to make it work again. This diff was extracted from #539, which at this point only basically contains the remaining fixes to ensure we can run stunreachability on mobile. Co-authored-by: Arturo Filastò <arturo@filasto.net> Co-authored-by: Arturo Filastò <arturo@filasto.net> * [backport] fix(oonimkall): run tests with InputOrStaticDefault policy (#634) Previous work to make ooni/probe#1814 possible has broken running stunreachability on mobile. This diff repairs the blunder and allows to run any experiment using InputOrStaticDefault with oonimkall. Diff extracted from #539. Co-authored-by: Arturo Filastò <arturo@filasto.net>
ainghazal
pushed a commit
to ainghazal/probe-cli
that referenced
this pull request
Mar 8, 2022
This commit introduces a new `InputLoader` policy by which, if no input is provided, we use a static default input list. We also modify the code to use this policy for dnscheck and stunreachability, with proper input. We also modify `miniooni` to pass the new `ExperimentName` field to the `InputLoader` to indicate which default input list to use. This diff is part of a set of diffs aiming at fixing ooni/probe#1814 and has been extracted from ooni#539. What remains to be done, after this diff has landed is to ensure things also work for ooniprobe and oonimkall.
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.
This commit introduces a new
InputLoaderpolicy by which, if noinput is provided, we use a static default input list.
We also modify the code to use this policy for dnscheck and
stunreachability, with proper input.
We also modify
minioonito pass the newExperimentNamefield tothe
InputLoaderto indicate which default input list to use.This diff is part of a set of diffs aiming at fixing
ooni/probe#1814 and has been
extracted from #539.
What remains to be done, after this diff has landed is to ensure
things also work for ooniprobe and oonimkall.