Add baseurl as parameter to BwScan so that other URLs can be used. #79
Conversation
|
Add baseurl as parameter to BwScan so that other URLs can be used.
Thanks for this pull request!
But we also need to be able to modify "bw_files" in the config:
* we might choose to add a 128 MB file, or remove the smaller files
* we might put different random data in the files on different bandwidth servers
(this is how the current bandwidth servers are implemented)
Can you make these changes in this pull request?
Or would you like to do a separate pull request?
|
juga0
force-pushed
the
feature/var_url
branch
from
14d2cb9
to
28a6aa4
Compare
|
@teor2345 it would probably be easier to specify the files in a configuration file rather than passing them by command line. See this other PR as an example of what i mean 553d5bb#diff-13462b18037a179e7f8b7eafaf14bfb3R9 |
|
@teor2345 it would probably be easier to specify the files in a configuration file rather than passing them by command line. See this other PR as an example of what i mean 553d5bb#diff-13462b18037a179e7f8b7eafaf14bfb3R9
What do you think?.
This looks like a good solution.
Help me understand why we want this PR #79?
I don't think it is possible to change the server without changing the hashes.
Also, the default bandwidth server that is currently in the code is a poor choice.
Using for testing changes the performance of one of the public bandwidth authorities.
https://trac.torproject.org/projects/tor/ticket/24090
Can you please remove it?
We will probably replace it with a CDN or a dedicated test server:
https://trac.torproject.org/projects/tor/ticket/21990
|
This PR would allow to specify a different server by command line, but not files, as it would require to provide also the hash, which seems lot of data to provide by command line. PR #76 would allow to specify a different server in a config file and also the files in the same config file. When any of these PRs has been merged, we can use other server (and files). Do you mean i should completely remove the server even as a default/example?. Do we have another we can use in the meanwhile at least for testing?.
When a CDN or dedicated test server could be ready?. |
I can provide a dedicated server to pull from if that helps. |
|
I can provide a dedicated server to pull from if that helps.
It's a physical (old) machine on a Sunet network, currently unused.
LMK if that'd help with testing at least.
Thanks, Linus, a test server would be helpful
|
|
This PR would allow to specify a different server by command line, but not files, as it would require to provide also the hash, which seems lot of data to provide by command line.
Or do you think we should also have the option to provide the files (size and hash) by command line?.
I don't mind exactly how it works: do whatever you think is best.
Here are our goals:
* We need to provide a default test config that works.
* We can't use existing production bandwidth servers, unless they are a CDN.
* We need users to be able to change the bandwidth server.
* We need users to be able to specify more than one server/file/hash set, for load balancing.
I suggest:
* configure bandwidth server addresses and allow disabling hash checking on the command line
* configure multiple server/file/hash sets in a config file
|
I've revived siv.sunet.se. The URL is http://siv.sunet.se/bwauth/FILE where FILE is any of
It has an old x509 cert sitting around. LMK if I should turn that into a more useful cert. |
It would be great if you can create a valid certificate (maybe use let's encrypt?), otherwise bwscanner will fail with |
Done. |
|
@ln5 awesome, thanks!. |
|
|
Great, thanks! |
|
Oh, are we still using SHA1?
I don't think that's a great idea for a new system.
Would it take much effort to use SHA256?
|
|
juga0
force-pushed
the
feature/var_url
branch
from
28a6aa4
to
6b2f2d3
Compare
i changed the default baseurl and file hashes to match the ones of @ln5.
Changed default baseurl and file hashes in the default config in #76. |
|
I didn't finish this branch, but might be worth looking at too: |
| @@ -76,7 +69,7 @@ def choose_file_size(self, path): | |||
| return max(self.bw_files.keys()) | |||
|
|
|||
| def choose_url(self, path): | |||
| return self.baseurl + '/' + self.bw_files[self.choose_file_size(path)][0] | |||
| return self.baseurl + self.bw_files[self.choose_file_size(path)][0] | |||
|
|
|||
There was a problem hiding this comment.
Is dropping the / intensional? Or perhaps should check that baseurl has a trailing slash in __init__.
There was a problem hiding this comment.
It is intentional. Yes, it seems a good solution to me to check the trailing slash in __init__.
IMO, it would be even better to move URLs and files to a config file.
There was a problem hiding this comment.
As it is done in https://github.com/TheTorProject/bwscanner/pull/76/files#diff-13462b18037a179e7f8b7eafaf14bfb3, though it needs to be updated with the new urls and files
|
This PR is now fixed in a different way by #105. |
No description provided.