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.

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

refactor(MeasuringNetwork): define dialers without resolver #1259

Merged
merged 4 commits into from
Sep 12, 2023

Conversation

bassosimone
Copy link
Contributor

@bassosimone bassosimone commented Sep 12, 2023

I initially planned on defining only the dialers with resolver and explicitly passing &NullResolver{} to them.

But I have just realized that, instead, defining the dialers without resolvers (which is what measurexlite needs anyway) will allow me to rewrite the without-resolver case to be very simple and completely ignore resolvers.

Once I have done this rewrite, we can keep the existing implementation for legacy code, but we can also have:

  1. the above-mentioned simple dialing implementation without resolvers that fullfills measurexlite needs;

  2. another implementation tailored to the needs of beacons.

So, voila, I think I now have a reasonable way forward to introduce beacons support in netxlite.

Part of ooni/probe#2531

I initially planned on defining only the dialers with resolver
and explicitly passing &NullResolver{} to them.

But I have just realized that, instead, defining the dialers
without resolvers (which is what measurexlite needs anyway) will
allow me to rewrite the without-resolver case to be very simple
and completely ignore resolvers.

Once I have done this rewrite, we can keep the existing
implementation for legacy code, but we can also have:

1. the above-mentioned simple dialing implementation without
resolvers that fullfills measurexlite needs;

2. another implementation tailored to the needs of beacons.

So, voila, I think I now have a reasonable way forward to
introduce beacons support in netxlite.

Part of ooni/probe#2531
internal/model/netx.go Outdated Show resolved Hide resolved
internal/model/netx.go Outdated Show resolved Hide resolved
@bassosimone bassosimone merged commit a76ac68 into master Sep 12, 2023
6 checks passed
@bassosimone bassosimone deleted the issue/2531 branch September 12, 2023 12:31
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this pull request Feb 13, 2024
I initially planned on defining only the dialers with resolver and
explicitly passing `&NullResolver{}` to them.

But I have just realized that, instead, defining the dialers without
resolvers (which is what measurexlite needs anyway) will allow me to
rewrite the without-resolver case to be very simple and completely
ignore resolvers.

Once I have done this rewrite, we can keep the existing implementation
for legacy code, but we can also have:

1. the above-mentioned simple dialing implementation without resolvers
that fullfills measurexlite needs;

2. another implementation tailored to the needs of beacons.

So, voila, I think I now have a reasonable way forward to introduce
beacons support in netxlite.

Part of ooni/probe#2531
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant