Rewriting the CalDAV plug from scratch #525
Conversation
libsecret is more modern than libgnome-keyring. It is not compatible with password integer identifiers so it has been changed to a string.
Add handling for datetime.datetime in Date class else because datetime.datetime implements datetime.date, the date is silently not converted and can cause difficult to find errors. Add a debug line to SyncEngine to more easily find errors from log
|
Hi! I just tried this PR today (again) and I am genuinely impressed, how well this works. While using it, some minor issues arose for me, but those should be relatively simple to fix:
For the last point, I got it to work by replacing the {
"name": "python3-caldav",
"buildsystem": "simple",
"build-commands": [
"pip3 install --use-deprecated=legacy-resolver --no-index --find-links=\"file://${PWD}\" --prefix=${FLATPAK_DEST} caldav --no-build-isolation"
],
"sources": [
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/4c/c4/13b4776ea2d76c115c1d1b84579f3764ee6d57204f6be27119f13a61d0a9/python-dateutil-2.8.2.tar.gz",
"sha256": "0123cacc1627ae19ddf3c27a5de5bd67ee4586fbdd6440d9748f8abb483d3e86"
},
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/da/ce/27c48c0e39cc69ffe7f6e3751734f6073539bf18a0cfe564e973a3709a52/vobject-0.9.6.1.tar.gz",
"sha256": "96512aec74b90abb71f6b53898dd7fe47300cc940104c4f79148f0671f790101"
},
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/80/be/3ee43b6c5757cabea19e75b8f46eaf05a2f5144107d7db48c7cf3a864f73/urllib3-1.26.7.tar.gz",
"sha256": "4987c65554f7a2dbf30c18fd48778ef124af6fab771a377103da0585e2336ece"
},
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/cb/38/4c4d00ddfa48abe616d7e572e02a04273603db446975ab46bbcd36552005/idna-3.2.tar.gz",
"sha256": "467fbad99067910785144ce333826c71fb0e63a425657295239737f7ecd125f3"
},
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/eb/7f/a6c278746ddbd7094b019b08d1b2187101b1f596f35f81dc27f57d8fcf7c/charset-normalizer-2.0.6.tar.gz",
"sha256": "5ec46d183433dcbd0ab716f2d7f29d8dee50505b3fdb40c6b985c7c4f5a3591f"
},
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/6d/78/f8db8d57f520a54f0b8a438319c342c61c22759d8f9a1cd2e2180b5e5ea9/certifi-2021.5.30.tar.gz",
"sha256": "2bbf76fd432960138b3ef6dda3dde0544f27cbf8546c458e60baf371917ba9ee"
},
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/e7/01/3569e0b535fb2e4a6c384bdbed00c55b9d78b5084e0fb7f4d0bf523d7670/requests-2.26.0.tar.gz",
"sha256": "b8aa58f8cf793ffd8782d3d8cb19e66ef36f7aba4353eec859e74678b01b07a7"
},
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/2c/4d/3ec1ea8512a7fbf57f02dee3035e2cce2d63d0e9c0ab8e4e376e01452597/lxml-4.5.2.tar.gz",
"sha256": "cdc13a1682b2a6241080745b1953719e7fe0850b40a5c71ca574f090a1391df6"
},
{
"type": "file",
"url": "https://files.pythonhosted.org/packages/97/b9/7d66fcc73bcc186ae018ee919f20498da86f095a632e4e26b3cebce584a3/caldav-0.8.0.tar.gz",
"sha256": "5b40dc2b39950e78989d515ce6a2c1131f20cc2c413ba28f8d5b582546b40a4c"
}
]
},(note: the legacy-resolver is necessary because of dateutil) Thanks for working on this! |
|
@eladyn thanks for the comment and the appreciation ! Sorry for not responding earlier, I'm just back from vacation. Lemme get back to you point per point:
It's a known issue. I noticed it a long time ago and I remember trying things with locks around the main entrypoints of the plugin to fix that, but in the end it didn't work. I'm guessing it's more of a problem of the machinery that execute the plugin which but, I should be working on that in another PR at the very least.
Noted, I'll fix it in the translation GTG => CalDAV by just protecting for such value to be uploaded. The more appropriate way would be to handle it in the main UI only when the plugin is active, but implication are deeper and it might take a while to be integrated.
Hum. That's a problem. I'm guessing the attributes I'm accessing on the task are somehow not up to date when
Spoiler : I don't know how to build flatpak x), I updated it manually just as a hint to maintainers for when this will be merged. But I'll definitely update the PR with yours ! Thanks a lot. Again thanks a lot, especially for the feedback ! |
|
Hi @jaesivsm o/ |
|
@diegogangl Yup, merged master a while back into the branch (hence the It's basically ready for a merge with master. NTH thingies mentioned somewhere above:
Both this things are not merge-blocking and could be solved in another PR. This review could still benefit a review by a core maintainer. Notably:
|
|
Awesome o/ I can merge this, what GH won't let me do is rebase and merge, which is how we usually merge PRs (easier for @nekohayo to make changelogs afterwards) |
|
|
||
| def get_gtg(self, task: Task, namespace: str = None): | ||
| "Extract value from GTG.core.task.Task according to specified getter" | ||
| return getattr(task, self.task_get_func_name)() |
There was a problem hiding this comment.
Why not call the getter straight from the task?
There was a problem hiding this comment.
I'm not sure what you mean. Could you give an example ?
The Field class is suppose to be a one attribute translator made destined to get and set attribute from task. Getters and setters are set up in task_get_func_name and task_set_func_name because most of the current Task API is made through getters and setters and no property. But that's a part I'm going to alter if the new_core branch gets merged before this one.
There was a problem hiding this comment.
I mean why not use task.get_status() or task.get_due_date() instead of going through getattr?
There was a problem hiding this comment.
It's just a way to have a declarative way to set field translator. Having a getter allows some overriding of the Field class.
|
Got this error when trying with a (probably) incorrect server url: |
|
Another one, when the server rejects you: |
|
All of those errors arose deep from the |
|
There isn't much in the way of a standard error dialog TBH. We should look into it once we get to Gtk4 + libadwaita and the new toasts. What we do have is a catch-all dialog that pops up when there's an uncaught exception. It seems like these errors aren't triggering it because they are coming from threads. Maybe you can Also could you send me the server url you used? I forgot to ask :) |
|
Seems like you need to tag tasks with Also, when I get this error when I set a due earlier than a start: Also, could you rebase on current master and squash/reword some of commits? We should keep history as clean as possible, otherwise making a changelog becomes challenging. |
Totally ! I'm not sure how though, could you point me toward the proper file to edit ?
(Hopefully) just fixed that :)
I try to keep up with master as often as I can. All the commit in there are worthless (and reaaaaaally entangled with the rest of the commits from master). It'd be simpler to merge squash in master. |
|
Def fixed, did you get a chance to look at the other errors? (from caldav) |
|
o/ @jaesivsm We should have a new page for caldav sync and a link to it in |
|
@jaesivsm feel free to tag me when you add the help page. I am starting to take a look at making any needed updates to the other areas of the user manual. There are a couple of sync pages that we had before. I am not sure if you want to use one as a starting point since it provides general sync services info and then you could get more specific with that: gtg-add-sync.page, gtg-sync.page, gtg-create-sync.page. They’re all in the location Diego noted above. I removed these pages from the manual last year since we no longer had sync services. We could then link any new ones in the Going Further or a new section in the index. We also need to remember to add any new pages to meson.build. |
|
I'd definitely be happier about Danielle taking care of the docs here indeed, as I am too stretched and unfocused right now to be able to commit to this, and I trust she would do a better & more attentive job at it than me! |
nekohayo
added
the
maintainability
Based on #419 but rewritten the entire backend. I tried to make smalls patches, mentioned them on #407 but finally started rewriting the entire thing.
CalDAV features support status in GTG :
RELATED-TOparametersPERCENT-COMPLETEbased on children statusesIN-PROCESSbased on children statusesPARENT/CHILDRENrelationshipsPARENTrelationship)DTSTARTCalDAV propertyDTSTAMPCalDAV propertySEQUENCECalDAV propertyCATEGORIESto davGTG.core.task.Task.content)General :
do_periodic_importsset_taskremove_taskNotes :
datetimeobjectGTG.core.task.Task.tidandGTG.core.task.Task.uuidon task creation