multitheftauto
New `ip2c` resource
Is your feature request related to a problem? Please describe.
The worktree gets dirty whenever we start the admin resource, because the admin resource occasionally generates a new IpToCountryCompact.csv. We can't delete that file (#221) because sometimes servers can't actually generate the file themselves (9e2005f05b007ceabbb63d2448f7735dd47dae57).
Other resources (the only non-default one is dxscoreboard) is unnecessarily dependent on admin to provide the country flags feature.
Describe the solution you'd like
TL;DR: We create a new ip2c resource without this data file in our repository. We generate the file when creating the main resources archive. dxscoreboard, admin, and admin2 will now auto-start ip2c if it exists, and function without country detection otherwise.
We can generate the IpToCountryCompact.csv file on the build server when creating our release archive—either by reimplementing the code in Python, or building some sort of Lua wrapper that implements some of MTA's native functions using publicly available rocks [0].
I'd prefer the latter solution instead of the former as then we don't need to duplicate the code or put effort into keeping it consistent. The question is how flexible the admin code currently is—hopefully this is not an issue as we just move ip2c to its own resource.
Instead of dxscoreboard and admin hard-failing when <include> can't find ip2c, we do some checks and manually start ip2c if it exists. Or we add an optional field to include instead of implementing the same logic everywhere. (Use case: what if the server author would like to keep ip2c loaded, but not have it auto-start when e.g. admin starts?)
Additional context
It would also be nice to use this as an excuse to GitHubify our build process here. It would be nice to move the mtasa-resources build task off the build server, and into GitHub Actions. /cc @ccw808
[0] Note that LuaRocks support will not be necessary inside MTA. It would only be used on the build server to generate the csv file.
Oh, looking at the code (why didn't I do this earlier?), admin just fetches the csv file from the MTA website. And I assume the MTA website just generates the csv using some script. It'd take just a couple seconds to add wget http://mirror.multitheftauto.com/mtasa/scripts/IpToCountryCompact.csv to the build steps.
See:
https://github.com/multitheftauto/mtasa-resources/blob/e51ed2fb1736c547ca0c7984d43a161d2c6872ac/%5Badmin%5D/admin/server/admin_ip2c.lua#L12-L14
It would be nice to move the mtasa-resources build task off the build server, and into GitHub Actions.
Good idea
To complicate things, admin2 uses an entirely different approach from admin. It ships with a bunch of XML files that have no update mechanism:
https://github.com/multitheftauto/mtasa-resources/blob/20b75b74e8ef0fc2ff4687f51b27e13bf04736a6/%5Badmin%5D/admin2/server/admin_ip2c.lua#L183-L206
To simplify things, perhaps we could:
1: Create ip2c as a wrapper for some free and publicly-available web API (ip2c.org would be a good candidate). This negates the need for any ip2c-related infrastructure on MTA HQ's side.
2: Refactor the ip2c functionality in admin, admin2, and scoreboard to use the ip2c resource's export functions
If we wait until after admin2 reaches feature parity with admin (which is not very far) we can skip refactoring admin and just throw IpToCountryCompact.csv into .gitignore for now.
I don't think having a dependency on a third party web site is a good idea for the official resources. We have no control over what may happen to it in the future. Having a dependency on MTA infrastructure is preferred because servers already rely on it.
I had some concerns about using a third-party at first too, but I think in this instance it is acceptable because:
- This particular third party appears to be pretty reliable. From their about page:
This service is FREE (LGPL) because we strongly support technology sharing. FREE does not imply low quality. Our uptime has been 99.95% since the beginning of 2009
- IP geolocation isn't (or shouldn't be) considered a critical feature, so if the provider goes down or returns inaccurate results isn't a big deal.
- We can always issue an API-compatible update later on to switch providers (including one hosted by MTA HQ) if needed in the future.
We can't issue a resource update. If the chosen site goes down or changes its API then it will affect all MTA servers using the ip2c resource
My naive idea is to route it thru MTA, and if the API goes down, then we can change it to a new one?
We can do this similarly to mtasa.com/api/. We could generate the XMLs on a cron.
My naive idea is to route it thru MTA, and if the API goes down, then we can change it to a new one?
That's what sort of happens now. I don't see what is wrong with the current system
Yes, I agree we should stick with MTA infrastructure.
Next step would be to investigate the difference between admin and admin2's IPC system and figure out why admin2 does something different, and what the benefits are. Then it's worth spending the time to update the MTA backend to deliver xml files instead of a single csv.
(Sorry for the delay, I kept typing this up and losing the text in browser reboots.)
I've created a PR that resolves the initial concern about the worktree. In the long term a separate ip2c resource is ideal/GitHub Actions setup is ideal. I understand the desire to stick with MTA's infrastructure. Which approach do we want to use though? admin's IpToCountryCompact.csv or admin2's XML files? I'm inclined to just stick with admin's approach since we know it works well.
I'm inclined to just stick with
admin's approach since we know it works well.
Sounds reasonable. We could leave admin2 alone and investigate its new approach later.
If there is someone who wants to do it, I don't think there is a reason to do that later.. my opinion is that admin's approach is the best, and admin2 is just outdated with plenty of things, including its approach to ip2c. Using .csv files under our current infrastructure is our best bet to keep country definitions updated, you can probably reuse some code from the admin ip2c auto-updater as well.
The reason I say "investigate its new approach later" is because:
- migrating to a new ip2c data format can be considered a completely separate workstream (with its own set of bugs)
- nobody has evaluated what admin2 does differently, maybe it uses a more performant better approach, so we should not just get rid of it
I looked around the differences in admin and admin2 and here are the differences i found:
-
adminresource is updating ip tables periodically whereadmin2is relaying on static*.xmlfiles. -
admin2resource is saving some memory by only loading what it needs.
besides that, admin2's code seems to be cleaner than admin.
Does
admin2not have an update mechanism for it's xml files?
Nope that functionality was added at a later point to admin1 (by ccw)