refactor(enginenetx): rename HTTPTransport to Network #1293
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
The HTTPTransport model only offers a CloseIdleConnections callback, which semantics is obviously that of closing idle connections (unsurprisingly). However, the struct I am slowly building inside the enginenetx package soon will need to write back statistics to disk using a key-value store. I don't think we should overload the CloseIdleConnections semantics to do this job, since the resulting code would be quite surprising. Therefore, I have decided to rename HTTPTransport to Network and make it represent all the network abstractions required by the OONI engine. In time, I will move extra functionality in there. For now, let us be happy that we can easily define a Close method (currently empty) for this type having the usual io.Closer semantics, i.e., that any resource opened by the type itself is released when calling this method. This diff does the following: - it renames the files; - the type; - the tests; - it introduces an empty Close method; - it adapts users of this code to use the new semantics; - it introduces an model.KeyValueStore argument for the Network constructor which we're going to use soon to persist statistics. Part of ooni/probe#2531
Murphy-OrangeMud
pushed a commit
to Murphy-OrangeMud/probe-cli
that referenced
this pull request
Feb 13, 2024
The HTTPTransport model only offers a CloseIdleConnections callback, which semantics is obviously that of closing idle connections (unsurprisingly). However, the struct I am slowly building inside the enginenetx package soon will need to write back statistics to disk using a key-value store. I don't think we should overload the CloseIdleConnections semantics to do this job, since the resulting code would be quite surprising. Therefore, I have decided to rename HTTPTransport to Network and make it represent all the network abstractions required by the OONI engine. In time, I will move extra functionality in there. For now, let us be happy that we can easily define a Close method (currently empty) for this type having the usual io.Closer semantics, i.e., that any resource opened by the type itself is released when calling this method. This diff does the following: - it renames the files, the type, and the tests; - it introduces a Close method that closes the Network's underlying transport's idle connections; - it adapts users of this code to use the new semantics; - it introduces an model.KeyValueStore argument for the Network constructor which we're going to use soon to persist statistics; - updates `CONTRIBUTING.md` to say how we do internal testing when the main tests body is external. Part of ooni/probe#2531
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.
The HTTPTransport model only offers a CloseIdleConnections callback, which semantics is obviously that of closing idle connections (unsurprisingly).
However, the struct I am slowly building inside the enginenetx package soon will need to write back statistics to disk using a key-value store.
I don't think we should overload the CloseIdleConnections semantics to do this job, since the resulting code would be quite surprising.
Therefore, I have decided to rename HTTPTransport to Network and make it represent all the network abstractions required by the OONI engine.
In time, I will move extra functionality in there. For now, let us be happy that we can easily define a Close method (currently empty) for this type having the usual io.Closer semantics, i.e., that any resource opened by the type itself is released when calling this method.
This diff does the following:
it renames the files, the type, and the tests;
it introduces a Close method that closes the Network's underlying transport's idle connections;
it adapts users of this code to use the new semantics;
it introduces an model.KeyValueStore argument for the Network constructor which we're going to use soon to persist statistics;
updates
CONTRIBUTING.mdto say how we do internal testing when the main tests body is external.Part of ooni/probe#2531