removed support for tor

[sdushantha]

Support for Tor has been removed. If users want to use Tor, it can be done using --proxy

[matheusfelipeog]

Perhaps, instead of removing it completely like this, wouldn't it be better to start notifying in the terminal that this feature is deprecated and will be removed in a future version? Just a smoother process for removing the feature so as not to catch users by surprise.

[ppfeister]

@matheusfelipeog This is part of the backlog for the bump to 0.15.0 and deprecation would be in the release notes alongside other planned breaking changes (i.e. importable module name changed for parity with package)

Thoughts on sufficiency?

Note that users can still use --proxy to route requests through a tor proxy (for the very few users that may exist)

[ppfeister]

@sdushantha Was this just a simple application of the patch or are there additional changes on top? Linting seems to fail but that's minor enough

[matheusfelipeog]

@matheusfelipeog This is part of the backlog for the bump to 0.15.0 and deprecation would be in the release notes alongside other planned breaking changes (i.e. importable module name changed for parity with package)

Thoughts on sufficiency?

@ppfeister Yes, I understand that. I think I wasn't very clear about the idea; I'll improve it:

1. In the next version, whether it is major, minor, or patch, we would add a small notification to be shown in the terminal whenever the user runs the command with the --tor or --unique_tor flag.
2. This message will inform the user that the feature will be discontinued in the future and that we recommend using the --proxy flag for this case.
3. In a future major update, we would remove all code related to these tor features.

We already do something similar to inform that there is a new update of Sherlock, for example.

This makes the transition smooth and doesn't surprise the users. And realistically, very few people read the release notes.

[matheusfelipeog]

... Linting seems to fail but that's minor enough

Apparently because of this:

sherlock/sherlock.py:205:5: F841 Local variable `underlying_request` is assigned to but never used

[sdushantha]

@sdushantha Was this just a simple application of the patch or are there additional changes on top? Linting seems to fail but that's minor enough

@ppfeister This was a simple application of the patch
As @matheusfelipeog has pointed out, the linting can be easily fixed. Will fix today

[ppfeister]

@matheusfelipeog I follow what you're saying. I lean slightly towards the "remove it now" side of the fence for this specific flag, but I'm not opposed to keeping it with a deprecation warning.

One thing I'd change, however...

If keeping it around for another version, I think we should adjust the import behavior to make it optional, as described in #2130. Prompt on first --tor run to install torrequest if not found. Bit of a middle ground between the "I'll never use this" majority, the not wanting to depend on a 6 year stale package with security concerns, and the occasional user who may have it in use still.

Thoughts?

[matheusfelipeog]

One thing I'd change, however...

If keeping it around for another version, I think we should adjust the import behavior to make it optional, as described in #2130. Prompt on first --tor run to install torrequest if not found. Bit of a middle ground between the "I'll never use this" majority, the not wanting to depend on a 6 year stale package with security concerns, and the occasional user who may have it in use still.

@ppfeister I agree. We can set it as optional in the pyproject.toml:

[tool.poetry.dependencies]
torrequest = { version = "^0.1.0", optional = true }

[tool.poetry.extras]
torrequest = ["torrequest"]

Users can install with:

pip install 'sherlock-project[torrequest]'

The remaining handling should be done in the code to avoid ModuleNotFoundError exceptions.

[ppfeister]

Changes approved for rel 0.16.0
PR assigned to new issue on 0.16.0 project board