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 IPinfo offlinedb, preparation for 5.4 #645

Merged
merged 22 commits into from
Aug 4, 2024
Merged

Conversation

sstidl
Copy link
Collaborator

@sstidl sstidl commented Jul 30, 2024

No description provided.

@adolfintel
Copy link
Member

@sstidl I've updated the docker files. It seems to work locally, but I don't have a server running docker that I can test it on through the internet right now. I'm mostly concerned about getIP, I can't test that properly locally. Can you give it a go?

@sstidl
Copy link
Collaborator Author

sstidl commented Jul 31, 2024

Test was ok. I didnt find how to see dark mode but location works with local db and api key

Can you read through
https://github.com/librespeed/speedtest/blob/ipinfo_offlinedb/doc_docker.md
and add the parts about the local db?

I can't do it right now.

@adolfintel
Copy link
Member

Done, I accidentally pushed a small fix to docker/ui.php along with the readme. Not the best git hygene, I know, but we're going to squash these commits anyway.

@adolfintel
Copy link
Member

I just did some testing and the inclusion of the mmdb library raised the minimum PHP version 8.0, I'll see if I can make it optional.

@adolfintel
Copy link
Member

I've added some fallback code in getIP. If you have a version of PHP <8 it only gives you the IP address.

@sstidl
Copy link
Collaborator Author

sstidl commented Aug 1, 2024

I've added some fallback code in getIP. If you have a version of PHP <8 it only gives you the IP address.

As PHP<8 is not supported anymore I don't think it's good to signalize we support anything that low
https://www.php.net/supported-versions.php

@adolfintel
Copy link
Member

So, I did a bit more testing yesterday and I'd say 5.4 is ready to go.

Before I make the release I want to update the quickstart videos, since they've been obsolete for a few years now, and make them for both debian and a generic docker install. I'll make these today.

I haven't tested the mssql database since that came in from a PR and I don't have a windows server machine. Do you have something to test this on @sstidl ?

As for the minimum PHP version, I'd agree with you but I've already done the fallback code and I know for a fact that we have some backend servers (not under my control) that regularly pull updates from this repo and use PHP 7, and I don't want to break them (yet).

I'll also get rid of the current wiki since it has the same stuff as doc.md and doc_docker.md

@sstidl
Copy link
Collaborator Author

sstidl commented Aug 3, 2024

I don't have easy access to an mssql server so, no can't test it at the moment. Maybe we just write that in the Readme that mssql is untested.
I would also like to look over the docker related md files before release so please wait for me.

@adolfintel
Copy link
Member

I updated the quick start video, I'm ready when you are @sstidl

@sstidl sstidl marked this pull request as ready for review August 3, 2024 16:15
@sstidl
Copy link
Collaborator Author

sstidl commented Aug 3, 2024

@adolfintel please look over the changes to the *.md files i did... i tried to reduce redundancy and described how to use other dbs in docker scenario. i didnt have the time to test a docker deployment with an external db so this still need more detail. anyway I think we have good reason to release :-)

@adolfintel
Copy link
Member

The changes in the documentation look good to me.

I didn't test all the external databases, only the sqlite one since it required no configuration, and it worked. The code seems straightforward enough though, I doubt it will give us issues.

I'll release 5.4 tomorrow morning.

@adolfintel adolfintel merged commit 420be5e into master Aug 4, 2024
1 check passed
@sstidl
Copy link
Collaborator Author

sstidl commented Aug 4, 2024

@adolfintel thank you for the release. Please always use versions with 3 parts. My docker build workflow depends on it. So I changed the release to 5.4.0 and tagged 5.4.0.
Restarted the workflow now

@adolfintel
Copy link
Member

Yeah I just noticed the problem about semver. This project never strictly followed that standard 😅 , I'll have to be more careful

@adolfintel
Copy link
Member

By the way, if you have time, there are a bunch of issues and PRs that can be closed as they're no longer relevant. This project really needs a cleanup before work on 6.0 can start.

@sstidl
Copy link
Collaborator Author

sstidl commented Aug 4, 2024

Yeah I just noticed the problem about semver. This project never strictly followed that standard 😅 , I'll have to be more careful

Unfortunately the workflow still fails. Don't know how to trigger a corrected run so the docker images with the correct tags are built.

By the way, if you have time, there are a bunch of issues and PRs that can be closed as they're no longer relevant. This project really needs a cleanup before work on 6.0 can start.

My vacation ends today so I don't think I can afford some time in the next week. Maybe next weekend.

@sstidl sstidl deleted the ipinfo_offlinedb branch August 4, 2024 14:04
@adolfintel
Copy link
Member

There's no rush, we can do it when we have time. I'll start working again in a few days as well.

I'll take a look at the failed workflow and see what I can do.

@sstidl sstidl linked an issue Aug 4, 2024 that may be closed by this pull request
@adolfintel
Copy link
Member

I had to redo the release. I tried modifying docker-publish.yml to add the workflow_dispatch event so I could trigger it manually but it didn't want to trigger on the 5.4.0 tag again so I just deleted the release, the 5.4 and 5.4.0 tags and recreated them.

@jk779
Copy link

jk779 commented Aug 5, 2024

Hi guys,

i've been a very bad boy and have been overwriting standalone.php in my docker-compose.yml to add some deep customizations.

I now realize that this was a bad idea :/ Any quick recommendations on how to easily migrate my standalone.php "fork" to the new structure? I'm not even sure what the right file location would be and i'm not too keen on reimplementing the changes to a more sophistocated and correct way because of no time 🙈 and on top of that i didn'teven use a version control to retrace my steps 🌚 (but out of curiosity - how would one do that "the right way"?)

@adolfintel
Copy link
Member

@jk779 if you're just going to use it in standalone mode, you can copy your old custom UI and replace docker/ui.php (which gets copied to index.php when the image is built)

@sstidl
Copy link
Collaborator Author

sstidl commented Aug 5, 2024

@jk779 please don't use a closed pull request for issues. I created a new issue for you.

@librespeed librespeed locked as resolved and limited conversation to collaborators Aug 5, 2024
@sstidl sstidl linked an issue Aug 5, 2024 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Using IPinfo IP to Country ASN database ISP Spectrum Returns "Unknown ISP" message
4 participants