Skip to content
This repository has been archived by the owner on Sep 7, 2023. It is now read-only.

Speed lower than 10% #7

Open
nicolastarzia opened this issue May 26, 2018 · 3 comments
Open

Speed lower than 10% #7

nicolastarzia opened this issue May 26, 2018 · 3 comments

Comments

@nicolastarzia
Copy link
Contributor

Sometimes my speed is lower than 10% that I contract.

It will be nice if the speed lower than 10% that user contract, send an twitter to @anatel.

Just for information: 😭
https://twitter.com/NicolastarziaN/status/1000489073755213825

diraol added a commit to diraol/my-internet-speed that referenced this issue May 27, 2018
@cuducos
Copy link
Owner

cuducos commented May 27, 2018

It will be nice if the speed lower than 10% that user contract, send an twitter to @anatel.

It will be nice, but including ANATEL in the logic of the program is something I strongly disagree. As I've just said in #8:

  1. I think the main reason I kept the tweet message as a configuration, not as part of the program itself – a strict separation of config from code – is so users decide what they are tweeting and whom they are tagging; and I wouldn't like to step out of this path.
  2. Adding explicitly ANATEL is a Brazilian thing and won't make sense elsewhere.
  3. AFAIK (and as far as it is documented) 10% is a rather arbitrary value (personally I like the idea of tagging ANATEL, but I would do it every time the ISP disrespects ANATEL's regulation (that is why it needs a variable MINIMUM_SPEED) and not only when it disrespects too much (because too much is too subjective indeed).

As I also said there, using general concepts such asMINIMUM_SPEED make the project independent of ANATEL or any country-specific regulations, making it reasonable for most countries of the world I guess. I think if anyone likes this feature we could differ between just below the minimum speed and critical speed, keeping configuration out of code:

  1. We add a CRITICAL_SPEED envvar that defaults to MINIMUM_SPEED
  2. We add a CRITICAL_TWEET envvar that defaults to TWEET
  3. In the tweet method we decide if we use minimum or critical ifs and tweet messages.

I discuss it further (and some better concepts for implementation) in #7 though.

@nicolastarzia
Copy link
Contributor Author

Perfect, and I agree with you, it's wrong if include ANATEL in the logic.

To solve this "problem" what do you think if we change the MINIMUM_SPEED and TWEET to a list of key/value (I think in python it's tuples)?
So we can configure N messages according the speed, turning this robot a little bit more dynamic.

Test case

Config Scenario:

Config minimum speed (thresold) Message
10 My internet is terrible, only {real_speed}. I contract {contract_speed}, what do you think @myisp?
40 Hey @myisp, I think that something wrong with my internet, I'm working with {real_speed}, but pay for {contract_speed}, merely {percentage}

Test:
Contract_speed = 100

Real Speed Twitter Message
50 No message
35 Hey @myisp, I think that something wrong with my internet, I'm working with 35Mbps, but pay for 100Mbps, merely 35%
20 Hey @myisp, I think that something wrong with my internet, I'm working with 20Mbps, but pay for 100Mbps, merely 20%
5 My internet is terrible, only 5Mbps. I contract 120Mbps, what do you think @myisp?

@cuducos
Copy link
Owner

cuducos commented May 27, 2018

To solve this "problem" what do you think if we change the MINIMUM_SPEED and TWEET to a list of key/value (I think in python it's tuples)?

Python can handle key/value pairs, but most of the simple and friendly data structures of config files and envvars handlers (shell envvars and .env, for examle) cannot.

So I think either we:

  • keep this configurations simpler by now to avoid making a setup that is aleady complex (due to theamount of envvars needed)
  • or a possible PR would replace the simple .env file for something more powerful such as .yml or .json (what would make us move from python-decouple to something different in order to load configuration)

But personally I would avoid this level of complexity to handle settings.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants