Tuesday, 2021-06-01

dcalisteHello chriadam, how are you ?06:59
chriadamhi dcaliste, I'm well thank you06:59
chriadamhow has your week been?06:59
dcalisteIt was great, thanks.07:01
dcalisteDidn't touch much code for Sailfish though ;) And I screwed up my SDK installation once again. Docker and myself are not good friends...07:01
dcalisteBut maybe we can discuss a bit the different possibilities to support alarm attachments in mKCal, see https://git.sailfishos.org/mer-core/mkcal/merge_requests/6507:04
dcalisteThis MR is adding support for component attachments by adding a new table. Attachments were previously supported for URI only, and saved as a string list in the component table.07:05
dcalisteThis is not convenient since attachment may be inline data and have additional parameters like the mime type.07:06
dcalisteThus the addition of a new table.07:06
dcalisteFrom the iCal RFC, ATTACH line can be added to VALARMs also, not only components.07:07
dcalisteThis is supported at the moment for URIs only by storing a string list in the alarm table.07:07
dcalisteFor the same reason, this is not convenient.07:08
dcalisteThere are three possibilities to add support for attachment for alarms :07:08
dcaliste- add now a alarmId column in the new attachment table, and depending if componentId or alarmId is not null, we know to which element the alarm is refering to.07:09
dcaliste- use negative values in attachment:componentId to refer to alarm ids and positive values to refer to component ids.07:10
dcaliste- create another table with the same layout as the attachments one but dedicated to alarm attachment.07:10
chriadam@flypig: did you get a chance to look at that one ^07:10
chriadammy first impressions of the options are:07:11
dcalisteHonestly, I don't have any idea which solution is better in term of maintenance or good practices.07:11
flypigSorry, I didn't I'm afraid.07:11
chriadam1) I like that the columns are separate, so it's clear what each field means, without "magic".  but I don't like that it will be difficult to enforce reference constraints (e.g. foreign key constraints on either componentId or alarmId ... tricky)07:12
chriadam2) I like that it's simple.  but I don't like "magic" e.g. just from looking at the db it's impossible ot tell that negative value means alarmId07:12
chriadam3) duplicating the entire table schema seems suboptimal, but allows for maximum reference enforcement, I guess...07:12
chriadampvuorela: did you have any thoughts about which might be best?07:13
chriadamI don't have strong feelings in any direction, honestly.07:13
dcalisteAbout reference enforcement, there is not any at all (no delete cascade and things like that) in mKCal. All is handled by hand in the C++ code. So, following the same spirit, it's not really an issue ;)07:14
chriadamalthough, one day, it might be nice to fix that..07:14
chriadamI'm inclined to lean toward (1) for now07:15
chriadamflypig: do you have an off-the-cuff opinion?07:15
flypigAh, sorry, I'd need a little time to catch up.07:16
pvuorelado we have any actual need for attachments in alarms? if not, could just skip?07:18
dcaliste2) can be handled also easily without too much code duplication by playing a little bit with the preprocessor. The reading and writing part is already working on a prepared SQlite statement, so the duplication would just be when preparing the statement, which is two to three lines. In case it's better to have clearer schema, then 2) is not that much of a pain.07:18
dcalistepvuorela, since we can resort on option 2) or 3), which means no modification to the schemas of the new attachments table, then we're safe indeed.07:19
dcalisteThis discussion was more about "is this new attachment table future-proof", listing how alarm attachment could be added in the future without requiring a database migration.07:19
pvuorelathough would be good if we had some real schema migration for the database instead of trying to fit new needs on the existing schema.07:21
chriadamwell, data migration is unfortunately issue-prone07:21
chriadambetter, if possible to avoid messy data migration steps, until schema breaks are needed, I suppose07:22
pvuorelaon kf5-calendarcore the alarm attachments seem to have different api anyway, not using Attachment.07:22
dcalistepvuorela, oh, nice point, I didn't noticed.07:22
pvuorelachriadam: true also, but in mkcal case we've never updated the schema. i have a feeling doing that on some point could have been better in the long run.07:23
chriadamok, so where does that leave us?  don't worry about trying to future-proof the schema of the new attachments table, currently?07:25
dcalisteSo keep it as it is proposed is enough to support KCalendarCore attachment API.07:25
pvuorelasounds good07:27
chriadamsomething which affects all MRs more generally: apparently branching for 4.2.x has been delayed by a little while07:28
chriadamand considering how important that release is, my preference would still be to delay merging things, until after the branching07:28
chriadambut that is quite a long delay :-(07:28
chriadami.e. another 10 days or so IIRC07:28
dcalisteAbout my other MRs, no problem with me.07:29
dcalisteConcerning this specific mKCal attachment stuff, I did it for people in OMP who needed it for internal reasons not disclosed ;)07:30
dcalisteSo you should see with them if it is in a hurry for them or not.07:30
chriadampvuorela: do you know if https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/86 is urgent?07:30
chriadamhmm, no bug reference07:31
pvuorelathat requires rethinking the whole architecture.07:31
pvuorelanot going forward in a week or two.07:31
dcalisteIndeed ;) that's why I was proposing mKCal#6507:31
dcalisteTo help a bit avoiding to store string hashtable contents !07:32
pvuorelayep, with mkcal pr it's at least possible to store attachments and expect to get same things back later.07:32
dcalisteSo if it's not in a hurry neither, no problem to wait for 4.2 branching, even if it is getting late.07:32
chriadamwhat is the conclusion?  are we fine to merge mkcal#65 before branching?  low risk, and potentially unblocks OMP?07:33
pvuorelaeven though doing the attachments properly in qml plugin + app might not absolutely require the new parts.07:33
pvuorelachriadam: as that didn't appear too risky, i think we could merge it even earlier. i'll need to check it again though.07:34
chriadamok.  thank you07:34
chriadamI'll leave it in your hands.07:34
dcalistepvuorela, I agree, duplicating the KCalendarCore::Attachment into nemo-qml-plugin-calendar can be done separatetly, assuming that the database is following.07:34
dcalistepvuorela, chriadam, there is one thing about mKCal#65 that I would like to get your opinion on : attachment custom properties...07:35
dcalisteThe RFC is saying that you can save as many custom properties as you want for each attachment.07:35
dcalisteThis is not supported by KCalendarCore at the moment.07:35
dcalisteBut if it is important for OMP, it could be added.07:36
pvuorelamaybe i wouldn't prematurely add new features, could be that don't end up needed at all.07:36
dcalisteAt the moment, MR#65 is not supporting it neither, but should we once again be future proof by adding a string list column in the new attachments table in case ?07:36
dcalisteAnd leave it not used at the moment.07:37
pvuoreladunno, until we add attachment usage we are are quite free to modify that part of the schema.07:38
pvuorelahey and doing the attachments properly. i'd maybe expect servicehandler.h api / plugins getting used.07:38
sashikknox_lbt:  can i get account for git.sailfishos.org?)07:40
dcalisteAs long as the code is not released, because, as it is done at the moment, on each database opening, the attachment table is created. So we cannot add a modified schema after a released was done, even if we didn't actually used the attachment code in that release.07:41
chriadamsashikknox_: err07:41
pvuoreladcaliste: simple to add some migration that even just recreates the whole table if it detects it's the old version.07:42
chriadamsashikknox_: https://forum.sailfishos.org/t/migrating-sailfish-os-git-repos-to-github/6141/2307:42
chriadamwell, I mean the main post, not the comment I linked to, probably07:42
dcalisteThat's true, but I'm not feeling good with table migration ;) Anyway, if you think it's allright like that, I trust your knowledge on this.07:43
dcalisteSo, pvuorela, we just need to agree on the migration lines on attachment reading in the MR#65 and that will be good !07:46
dcaliste(about supporting existing attachment URIs saved as string list)07:47
pvuorelasince we haven't used for anything, i wouldn't bother much.07:47
dcalisteI would not myself neither, but just in case some users have some attachments as URIs that they don't want to disappear on the server side. But I agree that the use case is very restricted and maybe not worth the four or five lines in this already over long routine.07:49
pvuorelathey would need quite a bit of custom code for that. and privileges given to the custom code to be able to even access the database.07:51
chriadamok, so, concretely, does dcaliste need to make any changes to that MR#65 right now, or at this point should he just wait to see what review comments you have, pekka?07:51
dcalisteWell, I'm not speaking on device. I'm speaking of a use case were they have added some attachment as URIs on their desktop for instance and are happy with. The event synced to the phone. The event is then modified on device to change let say the start time and synced back to the desktop. Then, the URIs would disappear on desktop. Before the MR they would have not...07:53
pvuorelabut do we have any sync code that touches the attachments at all?07:54
dcalistechriadam, beside this minor migration point, I think we agree to keep the MR as it is, supporting attachment for components (but without custom properties).07:54
dcalistepvuorela : yes we have. They are imported in the database and restored when exported into iCal format.07:54
pvuorelahm, right.07:54
dcalisteActually, they are imported only if they are URIs, and all other properties like the mime type are lost. Thus the new table addition.07:55
sashikknox_chriadam: need jusr wait when it available on github?)07:58
chriadamsashikknox_: my understanding is that you won't need to wait long at all.  but perhaps I am mistaken.  maybe lbt will still add new user accounts, but it seems unlikely at this point.07:59
chriadamof course, if you have some pressing need, I'm sure it can be accommodated07:59
*** frinring_ is now known as frinring08:01
sashikknox_chriadam: i just want fixes fir SDL2 now ) https://forum.sailfishos.org/t/bug-4-1-0-24-sdl2-cant-create-eglsurface-cant-return-sdl-getdevicedpi/665408:01
chriadamlbt: ViGe: ^ not sure where the migration is at08:02
chriadamdcaliste: well, I guess some more discussion is needed on that MR related to the migration08:03
dcalisteYes, we can continue with pvuorela on the MR comments.08:03
chriadamthanks very much.  was there anything else to discuss today?08:04
chriadamstill https://git.sailfishos.org/mer-core/mkcal/merge_requests/63 and https://git.sailfishos.org/mer-core/messagingframework/merge_requests/59 I suppose08:04
chriadamI will run qmf#59 unit tests this week08:04
chriadamfor mkcal#63 I tend to think it can wait until after branching08:05
dcalisteThanks chriadam for qmf#59 testing.08:05
chriadamaside from that i have no qualms with merging tha tone08:05
dcalisteabout mkcal#63, it's more about cleaning code, so it can definitely wait after branching.08:06
chriadamany other changes which need our attention at this stage?  or flypig / pvuorela did you have things to discuss today?08:06
dcalisteNo, thanks, that's all from my side.08:07
chriadamok, in that case, I hope you have a great week!  thanks again for your time and efforts!  it is much appreciated.08:08
chriadamgood night :-)08:09
* chriadam -> away08:09
dcalisteThanks, chriadam, pvuorela and flypig. Enjoy your week.08:10
*** sashikknox[m] is now known as sashikknox08:20
lbtsashikknox_:  msg me your emal/username and I'll set it up - git is going away RSN but OBS is around and you may want to use that as it improves08:56
ViGechriadam, sashikknox_: yeah, the git migration is happening really soon. But I wouldn't worry about it too much. In the worst case the MR just needs to be reopened on github, which is of course a little bit of extra work, but nothing compared to the actual implementation work.09:18
*** frinring_ is now known as frinring19:29
nshiellHi I have a problem updating my phone to the latest SailfishOS20:34
malnshiell: which device?20:39
Nico-old-defunctAnd what problem?20:40
nshiellAn XA220:42
nshiellI goto Sailfish OS Updates20:42
nshiellWhen I try to download the update I see:20:42
nshiellProblem with Store20:42
nshiellCould not determine size of update20:43
maldo you have any patches or things from openrepos?20:53
nshiellA few21:27
nshiellThe OKBoard keyboard21:28
nshiellI might have a few other things - apps basically21:28
Nico-old-defunctTry uninstalling the OKBoard temporarily at least21:35
Nico-old-defunctIt sometimes breaks upgrades21:35
Nico-old-defunctThere is also a "clear store data" in Sailfish utils, that may help?21:35
nshiellI'll give those ideas a shot21:36
malI think release notes mentions some apps from openrepos which might cause issues21:41
Nico-old-defunctThe list wasn't really up to date :D21:42
nshiellRemoved OKBoard, rebooted, seems not to have made a difference21:42
malNico-old-defunct: it's very difficult to make a full list, those are just what were noticed during testing21:44
Nico-old-defunctmal: Some of those were wrong though, at least when I upgraded, but maybe the notes got updated later, since the comments called them out :321:46
malNico-old-defunct: it's possible the apps were fixed21:51
Nico-old-defunctI thought the upgrade notes were just copied from the last release21:52
nshiellI have the Storeman app, I go into "Installed applications", I see a few things21:56
nshielllibpython3_7m1_0 looks interesting21:56
nshielland a few other things21:57
malhave you updated all of those the latest versions?22:03
nshiellCan updating be done with the Storeman app?22:17
nshiellDoes it do it automatically?22:18
malI mean updating the apps from within storeman22:32
oozboozare there field reports on xperia 10 II with the latest 64 bit image?22:38
*** nyov is now known as Guest7160822:42

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