Having implemented the weather support for Itinerary, rebasing that onto a more comprehensive framework would indeed be welcome :)
I haven't looked too deeply at the implementation or the API yet, most of the feedback below is based on things learned when implementing this for Itinerary. ## api.met.no license and ToS compliance The data is coming from the Norwegian Meteorological Institute, CC-licensed and with an API that doesn't require authentication, well documented and with a mailinglist for service changes and roadmap/disruption announcements. As far as implementing Open Data by organizations/public administration goes this is exemplary and unfortunately quite rare. So at the very least we should follow their license and terms of services as close as possible, in letter and in spirit. Not all of that can be ensured by the library, some of this needs to be handled by the application. In the latter case those requirements need to be clearly documented though. In particular I remember implementing the following aspects: (1) add a point of contact to the User-Agent header, in case your client misbehaves. I only see this done in one place, https://invent.kde.org/ libraries/kweathercore/-/blob/master/src/weatherforecastsource.cpp#L52, which looks suspiciously like a verbatim copy from Itinerary, without even adjusting the contact address. (2) Request throttling. The ToS seem to have been changed in wording since I last looked at this, but the requirement is still similar: Ensure we don't run unnecessary many queries. The Itinerary implementation uses a random interval between 120-150 minutes as lower bound, based on previous suggestions in the ToS. The already suggested daemon is one way to ensure this globally, however I'm very reluctant to suggest a daemon for this, considering the deployment issues this causes for e.g. Flatpak or APKs. Itinerary uses a simple file cache and file mtimes, something like this should also hold up in a multi-process setup with a bit of extra care I think. (3) Attribution, as required by the CC-BY license. This has to happen in the UI, but as a library user I need to know about this requirement, so this needs prominent mention in the docs I'd say. See https://api.met.no/doc/TermsOfService for more details. I also see other services used there I'm not familiar with, are we complying with their licenses and terms of services? ## Privacy considerations (1) Requested geo coordinates are passed to the online API as-is, ie. this is potentially leaking a high-resolution position of the user to the outside. We can't avoid this entirely obviously, but we can reduce the impact by reducing the coordinate resolution. Itinerary uses 1/10th of a degree for this, which unless you get close to the poles are areas of a few kilometers in size, ie. rather than leaking your home address it's only leaking your home town. (2) What does the sunrise API offer beyond the existing sunrise API we have in KF5::Holidays? The latter has the advantage of doing the entire calculation locally. See https://api.kde.org/frameworks/kholidays/html/ namespaceKHolidays_1_1SunRiseSet.html (3) Similarly, we do have a full offline implementation for timezone lookup by geo coordinates here: https://api.kde.org/kdepim/kitinerary/html/ namespaceKItinerary_1_1KnowledgeDb.html#ac409f468badee3c05438695009d7c67f (4) There are http: only requests send to api.geonames.org it seems. Similarly as with the compliance point, some of this can be argued to be the job of the using application. It would seem safer though is the library would try to avoid mistakes to begin with. ## Other observations (1) The license seems to be GPL-2.0-or-later, which is incompatible with the Frameworks license policy. See https://community.kde.org/Policies/ Licensing_Policy (2) The result types (DailyForecast, HourlyForecast, etc) might benefit from being Q_GADGETs and having Q_PROPERTYs added, so they can be directly consumed by QML? (3) binary compatibility measures seem to be missing in a number of places: https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B Regards, Volker On Montag, 21. Dezember 2020 07:16:09 CET hanyoung wrote: > KWeatherCore: https://invent.kde.org/libraries/kweathercore is a library for > querying weather forecast data. During the development of KWeather, we > found the need to have a weather library. KWeatherCore is the result of > extracting weather data fetching code from KWeather. I think having a > dedicated weather library can serve the following propose: - simplify the > KWeather code > - easier to develop a weather daemon > - potentially less code duplication across KDE > > Many of you may have already seen my previous email to kde-devel mailing > list. > Thank you for your constructive suggestions. Here are something I want to clarify: > > I would also propose to consider doing a demon instead, so different > > programs/processes all interested in weather forecast data could share the > > data > > The end goal is a daemon indeed, but we want to build the daemon upon the > library. This would give us flexibility in the future if we don't want a > daemon. At least KWeather and other projects can still benefit from the > code. > > but we want to make sure we don't end up with two things. > > I admit there are some overlapped functionalities. But KWeatherCore isn't > designed as a weather data provider as Weather DataEngine. Instead, it's > trying to be the building block of weather related applications. > KWeatherCore saves you the hassle of dealing with APIs, getting locations > and converting timezone. You can build a daemon with it, or you can use it > in your applications. For example, KItinerary and KWeather use the same > weather API, but don't share code. That means two code base to maintain. > Regarding the dynamic nature of online APIs, it's better to have one > library, so other applications don't need to be worried about their APIs > being depraved, and they aren't able to update it in time. > > Though not currently implemented, KWeatherCore does intend to have weather > alerts added. We hope it can be done in this Sok > https://community.kde.org/SoK/Ideas/2021#KWeather > With this bit added, then the work on weather daemon can be started. > > Regards, > Han Young
