scripts: Read suspicious hosts from a file instead of hardcoding #17823

sanjaykdragon · 2019-12-29T00:39:12Z

referring to: #17020
good first issue: reading SUSPICIOUS_HOSTS from a file.
I haven't changed the base hosts that were included in the original source, just made it readable from a file.

practicalswift · 2019-12-29T13:31:00Z

contrib/seeds/makeseeds.py

Nit: Slightly shorter using strip() and RAII:

with open("suspicious_hosts.txt") as f: SUSPICIOUS_HOSTS = {s.strip() for s in f if s.strip()}

Nit: Slightly shorter using strip() and RAII:

with open("suspicious_hosts.txt") as f: SUSPICIOUS_HOSTS = {s.strip() for s in f if s.strip()}

Didn't know that strip() would work in this case, just changed it

practicalswift · 2019-12-29T15:10:43Z

Please squash and use a more descriptive commit message + PR title :)

Suggestion: contrib: makeseeds: Read suspicious hosts from a file instead of hardcoding

Also you might want to sort the entries in contrib/seeds/suspicious_hosts.txt to make it clear where in the file to add new entries without causing unnecessary merge conflicts.

BTW, welcome as a contributor! :)

sanjaykdragon · 2019-12-29T15:26:41Z

Please squash and use a more descriptive commit message + PR title :)

Suggestion: contrib: makeseeds: Read suspicious hosts from a file instead of hardcoding

Also you might want to sort the entries in contrib/seeds/suspicious_hosts.txt to make it clear where in the file to add new entries without causing unnecessary merge conflicts.

BTW, welcome as a contributor! :)

what do you mean sort entries?
And thanks for the welcome, also just squashed the commits.

practicalswift · 2019-12-29T16:21:10Z

what do you mean sort entries?

Sort the lines in the file :)

sort -u -o contrib/seeds/suspicious_hosts.txt contrib/seeds/suspicious_hosts.txt in your shell, or M-x sort-lines in an editor built on the shoulders of matching parentheses :)

sanjaykdragon · 2019-12-29T16:32:22Z

what do you mean sort entries?

Sort the lines in the file :)

sort -u -o contrib/seeds/suspicious_hosts.txt contrib/seeds/suspicious_hosts.txt in your shell, or M-x sort-lines in an editor built on the shoulders of matching parentheses :)

Seems pretty pointless, but I did it anyways.

practicalswift · 2019-12-29T16:41:24Z

Sort the lines in the file :)
sort -u -o contrib/seeds/suspicious_hosts.txt contrib/seeds/suspicious_hosts.txt in your shell, or M-x sort-lines in an editor built on the shoulders of matching parentheses :)

Seems pretty pointless, but I did it anyways.

The reason to sort entries is that it makes it clear where in the file to add new entries in the future.

In unsorted files people typically append new entries to the end of the file which is likely to cause merge conflicts if two different PR:s add entries to the same file.

Thus sorting the file will help avoid future merge conflicts :)

Makes sense? :)

sanjaykdragon · 2019-12-29T17:19:23Z

Sort the lines in the file :)
sort -u -o contrib/seeds/suspicious_hosts.txt contrib/seeds/suspicious_hosts.txt in your shell, or M-x sort-lines in an editor built on the shoulders of matching parentheses :)

Seems pretty pointless, but I did it anyways.

The reason to sort entries is that it makes it clear where in the file to add new entries in the future.

In unsorted files people typically append new entries to the end of the file which is likely to cause merge conflicts if two different PR:s add entries to the same file.

Thus sorting the file will help avoid future merge conflicts :)

Makes sense? :)

Oh, I thought you mean that people were just idiots and didn't know how to append to a file. Thanks for the explanation!

paymog · 2020-01-04T09:04:11Z

ack b9eb76ece28b4e86b94ecad32f6b5fb35310a7ef

laanwj · 2020-01-04T13:24:11Z

Thank you!
I'm not even sure the suspicious hosts file should be in the repository, but that's another concern.
ACK b9eb76ece28b4e86b94ecad32f6b5fb35310a7ef

laanwj · 2020-01-04T13:51:28Z

A few linter issues:

Python's open(...) seems to be used to open text files without explicitly

specifying encoding="utf8":

contrib/seeds/makeseeds.py:with open("suspicious_hosts.txt") as f:

^---- failure generated from test/lint/lint-python-utf8-encoding.sh

contrib/seeds/makeseeds.py:23:1: W191 indentation contains tabs

contrib/seeds/makeseeds.py:30:1: E101 indentation contains mixed spaces and tabs

sanjaykdragon · 2020-01-04T16:13:10Z

A few linter issues:

Python's open(...) seems to be used to open text files without explicitly

specifying encoding="utf8":

contrib/seeds/makeseeds.py:with open("suspicious_hosts.txt") as f:

^---- failure generated from test/lint/lint-python-utf8-encoding.sh

contrib/seeds/makeseeds.py:23:1: W191 indentation contains tabs

contrib/seeds/makeseeds.py:30:1: E101 indentation contains mixed spaces and tabs

fixed, I think

fanquake · 2020-01-06T23:32:44Z

@sanjaykdragon Can you squash your commits.

…coding

practicalswift · 2020-01-07T11:09:42Z

ACK e1c582c -- diff looks correct

sanjaykdragon · 2020-01-12T17:54:24Z

@laanwj you mind merging this?

maflcko · 2020-01-13T18:59:24Z

contrib/seeds/makeseeds.py

-    "54.66.214.167", "54.66.220.137", "54.67.33.14", "54.77.251.214",
-    "54.94.195.96", "54.94.200.247"
-}
+with open("suspicious_hosts.txt", mode="r", encoding="utf-8") as f:


Pretty sure this tries to read the file in the pwd, not in this folder. Is the script required to run in this folder, so that pwd==this folder already holds for other reasons?

Pretty sure this tries to read the file in the pwd, not in this folder. Is the script required to run in this folder, so that pwd==this folder already holds for other reasons?

Not sure where this script is supposed to be run from.

It's supposed to be run from the folder it is in, at least according to the instructions in the README (that I always follow): https://github.com/bitcoin/bitcoin/blob/master/contrib/seeds/README.md

(would make sense to make this path configurable, though, for example through a command line option)

…ardcoding e1c582c contrib: makeseeds: Read suspicious hosts from a file instead of hardcoding (Sanjay K) Pull request description: referring to: #17020 good first issue: reading SUSPICIOUS_HOSTS from a file. I haven't changed the base hosts that were included in the original source, just made it readable from a file. ACKs for top commit: practicalswift: ACK e1c582c -- diff looks correct Tree-SHA512: 18684abc1c02cf52d63f6f6ecd98df01a9574a7c470524c37e152296504e2e3ffbabd6f3208214b62031512aeb809a6d37446af82c9f480ff14ce4c42c98e7c2

…ad of hardcoding e1c582c contrib: makeseeds: Read suspicious hosts from a file instead of hardcoding (Sanjay K) Pull request description: referring to: bitcoin#17020 good first issue: reading SUSPICIOUS_HOSTS from a file. I haven't changed the base hosts that were included in the original source, just made it readable from a file. ACKs for top commit: practicalswift: ACK e1c582c -- diff looks correct Tree-SHA512: 18684abc1c02cf52d63f6f6ecd98df01a9574a7c470524c37e152296504e2e3ffbabd6f3208214b62031512aeb809a6d37446af82c9f480ff14ce4c42c98e7c2

fanquake added P2P Scripts and tools labels Dec 29, 2019

practicalswift reviewed Dec 29, 2019

View reviewed changes

sanjaykdragon changed the title ~~script: makeseeds.py improvements~~ contrib/scripts: makeseeds: Read suspicious hosts from a file instead of hardcoding Dec 29, 2019

fanquake changed the title ~~contrib/scripts: makeseeds: Read suspicious hosts from a file instead of hardcoding~~ scripts: Read suspicious hosts from a file instead of hardcoding Dec 29, 2019

laanwj mentioned this pull request Jan 4, 2020

contrib: makeseeds.py improvements #17020

Open

11 tasks

fanquake added the Waiting for author label Jan 4, 2020

contrib: makeseeds: Read suspicious hosts from a file instead of hard…

e1c582c

…coding

fanquake removed the Waiting for author label Jan 7, 2020

maflcko reviewed Jan 13, 2020

View reviewed changes

laanwj merged commit e1c582c into bitcoin:master Jan 20, 2020

MarkLTZ mentioned this pull request Apr 4, 2020

Bitcoin PR tracking litecoinz-core/litecoinz#14

Closed

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

scripts: Read suspicious hosts from a file instead of hardcoding #17823

scripts: Read suspicious hosts from a file instead of hardcoding #17823

Uh oh!

Conversation

sanjaykdragon commented Dec 29, 2019

Uh oh!

practicalswift Dec 29, 2019

Choose a reason for hiding this comment

Uh oh!

sanjaykdragon Dec 29, 2019

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Dec 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanjaykdragon commented Dec 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Dec 29, 2019

Uh oh!

sanjaykdragon commented Dec 29, 2019

Uh oh!

practicalswift commented Dec 29, 2019

Uh oh!

sanjaykdragon commented Dec 29, 2019

Uh oh!

paymog commented Jan 4, 2020

Uh oh!

laanwj commented Jan 4, 2020

Uh oh!

laanwj commented Jan 4, 2020

Uh oh!

sanjaykdragon commented Jan 4, 2020

Uh oh!

fanquake commented Jan 6, 2020

Uh oh!

practicalswift commented Jan 7, 2020

Uh oh!

sanjaykdragon commented Jan 12, 2020

Uh oh!

maflcko Jan 13, 2020

Choose a reason for hiding this comment

Uh oh!

sanjaykdragon Jan 13, 2020

Choose a reason for hiding this comment

Uh oh!

laanwj Jan 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

practicalswift commented Dec 29, 2019 •

edited

Loading

sanjaykdragon commented Dec 29, 2019 •

edited

Loading

laanwj Jan 20, 2020 •

edited

Loading