| *** rdr_ is now known as rdr | 02:45 | |
| dcaliste | Good morning pvuorela, how are you ? | 07:00 |
|---|---|---|
| pvuorela | dcaliste: good morning. good here, how are you? | 07:01 |
| dcaliste | Yes fine, thank you. | 07:03 |
| dcaliste | Thank you for your comments on mkcal#59. | 07:04 |
| dcaliste | As you mentioned, this is just a minor part to the larger issue of multi-notebook incidences. | 07:05 |
| pvuorela | yea, i guess you were hoping to get that in regardless of the future work. | 07:05 |
| dcaliste | Indeed because, it tries to address the issue in a public routine. | 07:06 |
| dcaliste | With 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 Sellerie | 07:10 | |
| dcaliste | pvuorela, 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 |
| pvuorela | dcaliste: sorry, got distracted here for a moment. | 07:33 |
| dcaliste | No problem. | 07:33 |
| pvuorela | dcaliste: on a superficial check i didn't have immediate problems but such a big change i'll need to look more closely. | 07:34 |
| dcaliste | Sure, 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 |
| pvuorela | dcaliste: the commits before the last one could be something to consider having first. let's see. | 07:35 |
| dcaliste | Indeed, they are just multi-notebook enablers without change in the public API. | 07:36 |
| dcaliste | In 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 |
| dcaliste | As you prefer… | 07:37 |
| dcaliste | The 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 |
| pvuorela | hm, how would the new api be used outside mkcal? | 07:41 |
| dcaliste | That'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 |
| pvuorela | ok, 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::Extended | 07:47 |
| dcaliste | Calendar is not multi-notebook).. | 07:47 |
| dcaliste | Yes, 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 |
| dcaliste | I 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 |
| pvuorela | small 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 |
| pvuorela | and aim to get the purge part in soon. | 07:52 |
| dcaliste | Ok, thanks. I'll rebase mkcal#60 on top of the purge PR after that. | 07:53 |
| dcaliste | For you to get aan idea of the next step, I've created mkcal#61, introducing the CalendarStorage class and its SqliteCalendarStorage implementation. | 07:58 |
| dcaliste | I'm still working on it though (need to create a test for instance). | 07:58 |
| pvuorela | alright, thanks. | 07:59 |
| dcaliste | The 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 |
| pvuorela | ok | 08:00 |
| dcaliste | Looking at CalendarStorage API, you may see that it's very simple actually, very close to upstream CalStorage interface. | 08:00 |
| dcaliste | It'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 |
| dcaliste | Besides these additions, it's a very straight forward usage of the (new) SingleSqliteBackend API. | 08:05 |
| dcaliste | There 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 |
| dcaliste | The ExtendedNotebook::deleteNotebook() is becoming a CalendarStorage::erase() method though. Named like that, in case we go to multi-DB backends later on. | 08:08 |
| dcaliste | Still 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 |
| dcaliste | The 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 |
| dcaliste | So 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 |
| pvuorela | alright. i'll try to digest all that. | 08:20 |
| dcaliste | Thanks, 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/!