Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Removed general settings page (BREAKING CHANGE) #1464

Merged
merged 14 commits into from
Jun 6, 2024

Conversation

alexjustesen
Copy link
Owner

@alexjustesen alexjustesen commented Jun 6, 2024

📃 Description

Important

This PR includes breaking changes to how speedtest servers are selected and when the scheduler runs, action is required on your part.

This PR removes the "General Settings" page and moves it's configurations to environment variables.

🪵 Changelog

➕ Added

  • app:ookla-list-server command to list local servers
    • you can also pass a search criteria as the first argument app:ookla-list-servers "frontier"
  • DISPAY_TIMEZONE environment variable to change the displayed date and time in the UI
    • NOTE: if your database returns timestamps in your local time you can skip this and instead you should set APP_TIMEZONE to ensure timestamps are written correctly to the database

✏️ Changed

  • general settings have been moved to environment variables, check out the docs for more information on all environment variables
    • prune_results_older_than -> PRUNE_RESULTS_OLDER_THAN
    • speedtest_schedule -> SPEEDTEST_SCHEDULE
    • speedtest_server -> SPEEDTEST_SERVERS
    • time_format -> DATETIME_FORMAT
  • refactored random servers are selected when running speedtests

🔧 Fixed

🗑️ Removed

  • timezone and db_has_timezone settings, these have been replaced by DISPLAY_TIMEZONE
  • TimezoneHelper class, no longer needed
  • UpdateGeneralSettings command as this is no longer needed

@alexjustesen alexjustesen added the 🎉 feature New feature or request label Jun 6, 2024
@alexjustesen alexjustesen self-assigned this Jun 6, 2024
@alexjustesen alexjustesen marked this pull request as ready for review June 6, 2024 16:25
@alexjustesen alexjustesen added this to the v0.20.0 milestone Jun 6, 2024
@alexjustesen alexjustesen merged commit 8ed0cce into main Jun 6, 2024
1 check passed
@alexjustesen alexjustesen deleted the removing-general-settings branch June 6, 2024 16:55
@madukan
Copy link

madukan commented Jun 22, 2024

Oh.. I miss the UI!
Wouldn't it be wonderful if both UI and App-Settings (Docker environment variables) were present! UI (database) can always override the docker environment. That's why there is an Admin role! More secure that way because you don't need to goto infrastructure level to manage the configuration.

@jgwehr
Copy link

jgwehr commented Jun 26, 2024

Yeah, these changes are kind of baffling to me. I'm sure there's a good reason - but it's so odd to me that a container needs to be ran before you can set its own environment variables. Or can't alter the schedule without restarting the container.

@m4teh
Copy link

m4teh commented Jun 27, 2024

Could someone help me with the technical rationale that undoubtedly must exist for a change like this to get considered and merged? :\ I simply cannot understand this being seen as being good.

odd to me that a container needs to be ran before you can set its own environment variables

Or having to rely on someone's third party website to generate your system key

@alexjustesen
Copy link
Owner Author

alexjustesen commented Jun 28, 2024

Could someone help me with the technical rationale that undoubtedly must exist for a change like this to get considered and merged? :\ I simply cannot understand this being seen as being good.

odd to me that a container needs to be ran before you can set its own environment variables

Or having to rely on someone's third party website to generate your system key

Let me keep it simple...

I made the change because it enabled other features and that third party key is just your key. Generate it, regenerate it it's only dependant on you.

@jgwehr
Copy link

jgwehr commented Jun 28, 2024

For future readers, the website isn't the only source of the APP_KEY - I think it's just offered as a convenience. You can also generate a key within the running container via php artisan key:generate --show. I'd also hope/guess any base64 key would work.

Alex - I love what you've done with the app so I don't mean to bring shade your way. But, in at least this area the README + AppKey give the impression that "speedtest-tracker.dev" is providing a license instead of a convenience. Some clarification might assuage the self-hosters who are on high alert for good projects going commercial.

@madukan
Copy link

madukan commented Jun 28, 2024

I second what @jgwehr say. @alexjustesen, you're doing a great job with this tooling -- much needed one by the many!

@m4teh
Copy link

m4teh commented Jun 28, 2024

Could someone help me with the technical rationale that undoubtedly must exist for a change like this to get considered and merged? :\ I simply cannot understand this being seen as being good.

odd to me that a container needs to be ran before you can set its own environment variables

Or having to rely on someone's third party website to generate your system key

Let me keep it simple...

I made the change because it enabled other features and that third party key is just your key. Generate it, regenerate it it's only dependant on you.

More so confusion around why removing ability to control the tests and find servers is a forward move? So that now I have to run commands to dump what was gui interactive previously, 10x quicker and 10x easier until these changes.

If I put an older version in front of somebody. feature wise it makes perfect logical sense and is intuitive. If I put latest in front of them, they won't even be able to perform a test, and wonder what's going on.

Maybe I'm missing something but to me it's almost akin to speedtest.net front end shutting down and directing everyone to pull out speedtest-cli and run from commands. Can't see it being positive by doing away with simple graphical configuration options that used to exist

@madukan
Copy link

madukan commented Jun 28, 2024

Could someone help me with the technical rationale that undoubtedly must exist for a change like this to get considered and merged? :\ I simply cannot understand this being seen as being good.

odd to me that a container needs to be ran before you can set its own environment variables

Or having to rely on someone's third party website to generate your system key

Let me keep it simple...
I made the change because it enabled other features and that third party key is just your key. Generate it, regenerate it it's only dependant on you.

More so confusion around why removing ability to control the tests and find servers is a forward move? So that now I have to run commands to dump what was gui interactive previously, 10x quicker and 10x easier until these changes.

If I put an older version in front of somebody. feature wise it makes perfect logical sense and is intuitive. If I put latest in front of them, they won't even be able to perform a test, and wonder what's going on.

Maybe I'm missing something but to me it's almost akin to speedtest.net front end shutting down and directing everyone to pull out speedtest-cli and run from commands. Can't see it being positive by doing away with simple graphical configuration options that used to exist

I think the find servers is still in the CLI. Just that would be nicer still on the UI, because of the usability factor (no one wants to restart your Speedtest instance between the test-target server switching!).

UI provide dynamism/accessibility/usability aspects, while CLI provide better automation. Both are equally important!

@alexjustesen is there any chance getting the UI back? Keys I think as you mentioned can be passed through environment variable, so the DevOps guy really don't have to use the convenience service provided. Correct me if I'm wrong here.

@m4teh
Copy link

m4teh commented Jun 28, 2024

UI provide dynamism/accessibility/usability aspects, while CLI provide better automation. Both are equally important!

Of course, I agree. CLI and GUI aren't mutually exclusive. No intent to speak to that.

In a nutshell my confusion is—what is the thought process behind manufacturing unnecessary complication and difficulty (sudden obligation for running commands before anything works by default). Breaking both existing and future users' UX alike. Which had an intuitive, logical and no-debt expected means of configuration via admin panel? (simply click settings and type servers/schedules and that's it)

Can only ever result in blatant downside to adoption and usage. I personally updated to latest and had to take the time figuring out why my speedtest app just opted itself out of its sole job. With all my settings. And why the Settings feature bizarrely doesn't exist now. That was confusing to wonder. After browsing here, noted breaking changes and just simply opted to revert to the last functional version; avoid having to take further time figuring out commands, fixed variables and other things for that weren't necessary yesterday.

What is being gained from doing so? Maybe a better phrasing. Can't see how removing almost my entire purpose for a GUI does

@Cinj216
Copy link

Cinj216 commented Jun 30, 2024

I agree with everything said here. Nowhere near version 1 and this thing is already suffering from feature creep. Seems I can't go a month without noticing that the tracker has broke down and then I have to run to github and find out what breaking change for the sake of change he made this month. Ridiculous.

@OddSquirrel
Copy link

I get it, guys, I really do. I kept scratching my head a ton as well when everything got flipped around for no apparent reason.

Thing is, I'm not sure the tone of the discussion is conducive to motivating the maintainer. If it were me doing this in my spare time for free, I might be tempted to just tell everybody to eff off and pull the plug altogether. Just sayin'. ¯_(ツ)_/¯

@m4teh
Copy link

m4teh commented Jun 30, 2024

I get it, guys, I really do. I kept scratching my head a ton as well when everything got flipped around for no apparent reason.

Zero tone issues on my part, or from others in my perspective. Conversation and feedback is constructive.

At the end of the day, people care enough about the software and project to take the time to turn up and have the conversation. That's not unimpressive in and of itself.

Thing is, I'm not sure the tone of the discussion is conducive to motivating the maintainer. If it were me doing this in my spare time for free, I might be tempted to just tell everybody to eff off and pull the plug altogether. Just sayin'. ¯_(ツ)_/¯

If it was your project and that was your initial perception to what is undeniably supportive and open perspective. I'd be greatly disappointed. Albeit, somebody would just fork the project and carry on logically ¯_(ツ)_/¯

@OddSquirrel
Copy link

Zero tone issues on my part, or from others in my perspective. Conversation and feedback is constructive.

Yeah, calling his continued efforts ridiculous would count in my book, but hey—give him the whole nine yards for all I care.

Everybody perceives dialogue differently, and you're welcome to gamble on the possibility that somebody would actually consider this constructive and friendly feedback. I just left my highly subjective $0.02 that may be taken or ignored at your leisure, and I have no stakes in this project other than a concern to see it go poof.

@Cinj216
Copy link

Cinj216 commented Jun 30, 2024

Zero tone issues on my part, or from others in my perspective. Conversation and feedback is constructive.

Yeah, calling his continued efforts ridiculous would count in my book, but hey—give him the whole nine yards for all I care.

Everybody perceives dialogue differently, and you're welcome to gamble on the possibility that somebody would actually consider this constructive and friendly feedback. I just left my highly subjective $0.02 that may be taken or ignored at your leisure, and I have no stakes in this project other than a concern to see it go poof.

If he can't take criticism which seems to be my experience with 99% of the fragile developers on GitHub, then that's his problem not mine. I'm not calling his efforts ridiculous, only the fact that he needlessly breaks the tracker seemingly every month for reasons known only to him while making it more and more user-unfriendly when it should be the opposite. So sorry I'm giving him honest feedback instead of just kissing his butt. Maybe I'll donate him some money when the development is progressing in a positive direction, then I'll be allowed to have an opinion?

@OddSquirrel
Copy link

If he can't take criticism which seems to be my experience with 99% of the fragile developers on GitHub, then that's his problem not mine. I'm not calling his efforts ridiculous, only the fact that he needlessly breaks the tracker seemingly every month for reasons known only to him while making it more and more user-unfriendly when it should be the opposite. So sorry I'm giving him honest feedback instead of just kissing his butt. Maybe I'll donate him some money when the development is progressing in a positive direction, then I'll be allowed to have an opinion?

Well, that is a nice work-up on informal fallacies, but me having an opinion doesn't preclude you from having one as well. I think the planet is large enough to house two or more different opinions, but you are welcome to differ.

Also, nobody ever came up with ideas like kissing anybody's behind or donating money to be able to voice your view. In fact, you did just that for free and, funnily enough, nobody stopped you.

I also don't know if people are 'fragile' when they don't cater to everybody's whim in their spare time just to get called out for it. Fragile may instead fit someone who likes to loudly voice his point of view but can't handle even mildly questioning feedback.

You know what he's free to do? He can start and stop this project as he pleases, put it on its head, or delete the repository and go to the movies with his partner instead of arguing his every move. He obviously had something he couldn't get around without moving settings externally. Big deal.

Now, if you don't mind, let me put that block function to good use. The Internet is a huge place, and I'm sure you'll find some other poor soul to argue with. 👋

@madukan
Copy link

madukan commented Jul 1, 2024

Whoa... I think we're getting to the wrong track here. It is quite simple than this way we chat (war/protesting not necessary).
Like a good bunch of scientists, we must just see the way we can find agreement through fact finding.

We can already see many of you who took a moment to take part already has a love for the product. That's why you're here!
I have to admit that I haven't myself looked through all the possibilities in this tooling yet.

Perhaps @alexjustesen should lead this discussion I would think. It is his project. He knows the best. Thanks to @alexjustesen when the other projects like this went offline, I was still able to provide the statistics my internet provider asks during their adjustments in the local networks/hubs. They appreciated it very much.

@mpaw
Copy link

mpaw commented Jul 9, 2024

@alexjustesen Can you provide some more details around why the settings page was removed? Maybe we can help resolve the issue that caused you to remove it. You spend significant time on this project so I assume you want others to use it. Having the settings in the web ui will make this app more accessible to less tech savvy users.

@Cinj216
Copy link

Cinj216 commented Jul 13, 2024

Now, if you don't mind, let me put that block function to good use. The Internet is a huge place, and I'm sure you'll find some other poor soul to argue with. 👋

Just the type of snowflake I'm talking about. Start the argument and then start blocking people when you can't handle the response. Keep yourself safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Time Zone issues after updating to v0.16.x
7 participants