Revert "Add timeouts to sniper source config"

[YvesHenri]

Reverts #5624

[YvesHenri]

I'll continue the discussion on here, so others can take a look at what has been done.

@Gobberwart Unfortunatelly theres no label close to "Unapproved", "Disagree" or even "I've been explaining this LOTS of times".

I did not only design this, but I've also searched for as many sniping sources as I could and tested it hardcore. I couldn't find too many sources, as we can see in the example file, but this is pretty much the purpose I made this task for. You can find the ones that best suits you and decide whether to share this precious information with others or not.

Everyones been complaining about pokewatchers, not only you. I tried my best to support people with this task on slack these past days, since its a new thing. People can barely find out that "source is not enabled" is not really an error, nor HOW can they solve this. I can't even imagine people setting their own custom timeouts for a reason. Mostly everyone that requests/modifies the timeout value are devs, if not all, but still you guys cant realize this (pokewatchers's issue).

For the last time, pokewatchers is horrible. They have the "Sucuri Firewall" and this is probably why it takes forever to load. You can choose a 10, 20 or even 30 seconds timeout and it will still timeout frequently. There's nothing you can do. The higher this value is, the longer it'll keep the bot iddle untill it timeouts/replies. This is far from a solution. I am not even going to mention that the more sources you have and the higher the timeout values are, you might end up with a 30-60 seconds of an iddle bot. I hope you know what this can lead to.

You shouldn't worry about this, since pokesnipers gives you pretty much all you need. You have some many targets that you even constantly run out of balls.
What's next? Add 100 other params and/or turn it into a "walking sniper" (MoveToMapPokemon)? No, thanks.

[DBa2016]

Okay, now help me understand why adding the possibility of the customization is so bad. If somebody wants to have it customizable - let him do it. If somebody wants to proceed with defaults - he has the same behavior as the hardcoded values.

I strongly disagree there is a need to revert this snipe-timeout-configurability unless it has severe errors (and if it has some, they should be corrected, not just removed).

@YvesHenri the label you are looking for is "Second review required". If a "unapproved" or "disagree" label existed, it would already be tagged to this "revert" PR. :)

[Gobberwart]

Revert is not required. It does not cause errors and was approved by multiple people. @YvesHenri You may not like it, but the whole point of a community project is to accept ideas/input from other people, not to tell them they're wrong because it doesn't line up with your original idea. I've explained why source timeouts are a useful feature, tested thoroughly, submitted my PR and had it approved. Just let it go man :)

[julienlavergne]

Beside this specific case (cannot tell if justified or not), I am not for allowing more parameters for the only reason that it add possibility without introducing bugs. Because most of parameters would fall into that category, so that is pretty much the end of a clear, simple and usable configuration file.

If a parameter is introduced to hide/workaround a deeper issue, then we should work on fixing that issue. if we cannot, we should make it clear that the parameter is temporary until issue is fixed.

If one person want something customizable, we cannot assume that all the others who did not voice out are ready to take a hit on the complexity of their config.
And to be honest, I hear much more complaints about the complexity of the config rather than its lack of parameters...

But again, I know nothing for that specific parameter :)

[YvesHenri]

Can't reply now. I'll edit this post anytime soon.

[Gobberwart]

@anakin5 Agree with you on the complexity, but I think this one is warranted. It's just a per-source timeout which doesn't even need to be set if the user doesn't want to (it will default to 5s). It's just useful if I'm using a source that I know will take more than the default 3s/10s to respond, so I can allow that source more time if I want to.

It's not hiding/working around something, it's just that some sources regularly take a long time to respond, so if a user wants to keep that source, the timeout has to go up or it'll fail 95% of the time.

In particular, that 3 second timeout is far too low for a lot of potential sources, and as it is, cannot be changed without modifying the code.

[julienlavergne]

Even 3 seconds might be a problem if we query the sources in the main thread. Do we have one thread per source ?
If sources are in separate thread, we can start with 1s timeout, multiply by 2 if it fails, and repeat.

[Gobberwart]

At 1s, almost nothing will respond. Unfortunately these sources are generally hosted poorly, so response time can fluctuate wildly. It really should be up to the user if they want to allow a given source a longer timeout or not.

[Gobberwart]

OK looks like this isn't going to happen. Closing.