New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

services/vdirsyncer: init #3912

Open

RaitoBezarius wants to merge 1 commit into nix-community:master from RaitoBezarius:vdirsyncer

Member

RaitoBezarius commented Apr 24, 2023 •

edited

Loading

Description

This introduces a vdirsyncer service I use in my configuration.

It has some shortcomings:

it is very locally scoped, so running vdirsyncer discover requires you to get the configuration file, I fix this by having a vsync alias in my configuration with the proper configuration. It can also run vdirsyncer discover by itself, but I think it requires interactivity, so it is annoying.

Checklist

Change is backwards compatible.
Code formatted with ./format.
Code tested through nix-shell --pure tests -A run.all.
Test cases updated/added. See example.
Commit messages are formatted like
```
{component}: {description}

{long description}
```
See CONTRIBUTING for more information and recent commit messages for examples.
If this PR adds a new module
- Added myself as module maintainer. See example.
- Added myself and the module files to .github/CODEOWNERS.

RaitoBezarius requested a review from rycee as a code owner

April 24, 2023 12:06


          services/vdirsyncer: init

c80bc5e

RaitoBezarius force-pushed the vdirsyncer branch from ef3f9fe to c80bc5e Compare

April 24, 2023 12:19

RaitoBezarius mentioned this pull request

Calendar and contact infrastructure #2335

Closed

Collaborator

teto commented May 1, 2023

might be nice to add a test to easily see how to use it. It doesn't use any "contact" infrastructure but that's something that we want to support later right ?

mdarocha commented May 4, 2023

FYI, the "unknown format" that vdirsyncer uses is configobj

Member Author

RaitoBezarius commented May 4, 2023

FYI, the "unknown format" that vdirsyncer uses is configobj

Thank you!

Member Author

RaitoBezarius commented May 4, 2023

might be nice to add a test to easily see how to use it. It doesn't use any "contact" infrastructure but that's something that we want to support later right ?

Yes and yes!

ncfavier reviewed

View reviewed changes

modules/services/vdirsyncer.nix

+              with lib;
+              let
+                cfg = config.services.vdirsyncer;
+                formatToUnknownFormat = settings:

Member

ncfavier May 7, 2023

Looks like the format they use is basically INI but with JSON values... https://github.com/pimutils/vdirsyncer/blob/079a156bf82fe86173d0d05ec0937703b5c6a735/vdirsyncer/cli/config.py#L151

So like,

settingsFormat = {
  type = types.attrsOf (types.attrsOf (pkgs.formats.json {}).type);
  generate = name: value: pkgs.writeText name (lib.generators.toINI {
    mkKeyValue = lib.generators.mkKeyValueDefault {
      mkValueString = builtins.toJSON;
    } " = ";
  } value);
};

modules/services/vdirsyncer.nix

+              in {
+                options.services.vdirsyncer = {
+                  enable = mkEnableOption "synchronization of calendars";
+                  settings = mkOption { type = types.attrs; };

Member

ncfavier May 7, 2023

Needs a description, default value and better type (see above)

modules/services/vdirsyncer.nix

Comment on lines +27 to +38

+                  configFile = mkOption {
+                    type = types.path;
+                    default = cfgFile;
+                  };
+                  onBootSec = mkOption {
+                    type = types.str;
+                    default = "15min";
+                  };
+                  onUnitActiveSec = mkOption {
+                    type = types.str;
+                    default = "30min";
+                  };

Member

ncfavier May 7, 2023

These need descriptions. I'd also get rid of configFile for now, unless there's a good reason to have it.

sumnerevans reviewed

View reviewed changes

modules/services/vdirsyncer.nix

+                  pkgs.writeText "vdirsyncer.conf" (formatToUnknownFormat cfg.settings);
+              in {
+                options.services.vdirsyncer = {
+                  enable = mkEnableOption "synchronization of calendars";

Member

sumnerevans May 8, 2023

Suggested change

      
                enable = mkEnableOption "synchronization of calendars";
          
                enable = mkEnableOption "synchronization of calendars via vdirsyncer";

modules/services/vdirsyncer.nix

+                    Install.WantedBy = [ "default.target" ];
+                    Service = {
+                      ExecStart = "${pkgs.vdirsyncer}/bin/vdirsyncer -c ${cfgFile} sync";

Member

sumnerevans May 8, 2023

We probably want to do metasync as well as sync.

Member

sumnerevans May 14, 2023

as mentioned here: #3982 (comment)

we should probably do vdirsyncer discover before sync and metasync

ncfavier mentioned this pull request

vdirsyncer: add module #3982

Closed

7 tasks

ncfavier mentioned this pull request

Add infrastructure for contacts and calendars #4078

Merged

6 tasks

ncfavier mentioned this pull request

vdirsyncer: synchronize metadata as well #4167

Merged

6 tasks

stale bot commented Aug 12, 2023

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
If it is unfinished but you plan to finish it, please mark it as a draft.
If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

stale bot added the status: stale label

Collaborator

teto commented Aug 12, 2023

we already have the service. I dont close in case you want to merge missing features .

Member Author

RaitoBezarius commented Aug 12, 2023

we already have the service. I dont close in case you want to merge missing features .

Will take a look and see what I can do.

stale bot removed the status: stale label

stale bot commented Nov 10, 2023

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
If it is unfinished but you plan to finish it, please mark it as a draft.
If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

stale bot added the status: stale label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment