Tuesday, 2023-05-09

*** rdr_ is now known as rdr02:45
dcalisteGood morning pvuorela, how are you ?07:00
pvuoreladcaliste: good morning. good here, how are you?07:01
dcalisteYes fine, thank you.07:03
dcalisteThank you for your comments on mkcal#59.07:04
dcalisteAs you mentioned, this is just a minor part to the larger issue of multi-notebook incidences.07:05
pvuorelayea, i guess you were hoping to get that in regardless of the future work.07:05
dcalisteIndeed because, it tries to address the issue in a public routine.07:06
dcalisteWith it, at least, one can restructure further internally without external disruption. But for this routine, in my opinion, the API is deficient (my fauklt actually, I introduced it a while ago).07:06
*** Sellerie2 is now known as Sellerie07:10
dcalistepvuorela, what do you think about the proposed API in mkcal#60 ? It's creating a class exposing a multi-notebook serialiser/deserialiser into a single SQLite backend. It's basically taking all the implementation parts of the SqliteStorage, but trying two things:07:18
dcaliste- having a clear multi-notebook API,07:18
dcaliste- don't enforce the memory storage, just returning list of incidences, or taking list of incidences.07:19
pvuoreladcaliste: sorry, got distracted here for a moment.07:33
dcalisteNo problem.07:33
pvuoreladcaliste: on a superficial check i didn't have immediate problems but such a big change i'll need to look more closely.07:34
dcalisteSure, most of the code in singlesqlitebackend.cpp is coming from sqlitestorage.cpp. What do you think about the API itself ? From singlesqlitebackend.h.07:35
pvuoreladcaliste: the commits before the last one could be something to consider having first. let's see.07:35
dcalisteIndeed, they are just multi-notebook enablers without change in the public API.07:36
dcalisteIn the case of unique event UID in the DB, providing the notebook UID is redundant with the event UID though. So they may not make sense by themselves.07:37
dcalisteAs you prefer…07:37
dcalisteThe purpose of singlesqlitebackend.h is to create an API that allow multi-notebook. Later on, SqliteFormat can be adjusted to raise the UNIQUE constrain and extend it to include notebook UIDs also.07:38
pvuorelahm, how would the new api be used outside mkcal?07:41
dcalisteThat's a good question. At the moment, I see two usages of mKCal API:07:42
dcaliste- in Buteo sync plugins, there (besides the detection of notebooks), everything is dealing with one notebook only. So I'm currently developping a mKCal::CalendarStorage API allowing a representation of a single notebook into a KCalendarCore::MemoryCalendar. It would replace mKCal::ExtendedStorage there. And it is using the singlesqlitebackend internally.07:44
pvuorelaok, so that's sort of next step on top of the current PR?07:46
dcaliste- in nemo-qml-plugin-calendar, where access are multi-notebooks. There mKCal::ExtendedStorage must be replaced with something that would allow multi-notebook. I'm still undecided though if I put a singlesqlitebackend in calendarworker and let the manager stores the multi-notebook data or if I need to make ExtendedStorage multi-notebook aware (which is complicated by design since anything that inherit KCalendarCore::Calendar, like mKCal::Extended07:47
dcalisteCalendar is not multi-notebook)..07:47
dcalisteYes, the mKCal::CalendarStorage for single notebook access in a multi-notebook DB is the next step. Almost ready for review, currently in my single branch.07:47
dcalisteI decided to split the work so you can comment and review the new API / ideas as logical steps instead of getting the full rework at once.07:48
pvuorelasmall chunks is good, though in this case might be also good if i have the next step to check. but i'll dig deeper into this.07:51
pvuorelaand aim to get the purge part in soon.07:52
dcalisteOk, thanks. I'll rebase mkcal#60 on top of the purge PR after that.07:53
dcalisteFor you to get aan idea of the next step, I've created mkcal#61, introducing the CalendarStorage class and its SqliteCalendarStorage implementation.07:58
dcalisteI'm still working on it though (need to create a test for instance).07:58
pvuorelaalright, thanks.07:59
dcalisteThe pending issue for me is about the alarm handling. I'm wondering if the base class (namely CalendarStorage) is the best place to put it, like in ExtendedStorage, or if it's better to put it into SingleSqliteBackend…07:59
dcalisteLooking at CalendarStorage API, you may see that it's very simple actually, very close to upstream CalStorage interface.08:00
dcalisteIt's adding very few methods:08:00
dcaliste- load(uid), which is required in sync plugins to avoid reloading all the calendar into memory when dealing with changes,08:01
dcaliste- deletedIncidences(), since upstream does not support incidence marked as deleted,08:02
dcaliste- purgeDeletedIncidences(), to get rid of them when necessary,08:03
dcaliste- notebook() which is a getter since upstream Calendar doesn't have all the options that mKCal::Notebook supports.08:04
dcalisteBesides these additions, it's a very straight forward usage of the (new) SingleSqliteBackend API.08:05
dcalisteThere are some small divergence with ExtendedStorage though, like the fact that the notebook data are saved automatically when save() is called, Which saves additional API like ExtendedStorage::updateNotebook() and is closer to upstream APIs I think.08:07
dcalisteThe ExtendedNotebook::deleteNotebook() is becoming a CalendarStorage::erase() method though. Named like that, in case we go to multi-DB backends later on.08:08
dcalisteStill to give you a full picture, the second point of mKCal usage is the multi-notebook nemo-qml-plugin-calendar. It is appealing to plug the SingleSqliteBackend into the worker and let the manager stores the data in memory. It is solving the current memory duplication but it is raising one major issue at the moment:08:13
dcalisteThe worker needs to compute recurring event occurrences for every ranges. And this is a KCalendarCore::Calendar operation. Which is costly, so better leaving in the worker thread. So removing Calendar support from the worker thread is not yet possible. But at the same time, it must be done in a way since KCalendarCore::Calendar is not multi-notebook compatible…08:15
dcalisteSo that's the major blocker I see at the moment. My plan is still to go for CalendarStorage with single notebook support in sync storages and keep nemo-qml-plugin-calendar as it is.08:16
dcaliste(sorry in sync plugins, not storages)08:16
pvuorelaalright. i'll try to digest all that.08:20
dcalisteThanks, it's a large amount of information, I know.08:21

Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!