Skip to content

feat(jmap)!: re-export caldav.jmap from the standalone calendaring-jmap package - #711

Open
SashankBhamidi wants to merge 8 commits into
masterfrom
jmap-reexport-shim
Open

SashankBhamidi wants to merge 8 commits into
masterfrom
jmap-reexport-shim

Conversation

@SashankBhamidi

@SashankBhamidi SashankBhamidi commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • caldav/jmap/ no longer carries its own copy of the JMAP client; it now re-exports the standalone calendaring-jmap package (v1.1.0), added as an optional dependency (caldav[jmap]).
  • from caldav.jmap import JMAPClient still works, but now emits a DeprecationWarning pointing at the new canonical from calendaring_jmap import JMAPClient.
  • get_jmap_client()/get_async_jmap_client() keep resolving configuration through caldav.config.get_connection_params() exactly as before.
  • JMAP errors stay catchable as DAVError, via a conditional import on calendaring-jmap's side (feat: conditionally subclass caldav's DAVError when caldav is present pycalendar/calendaring-jmap#43, released as 1.1.0).
  • Docs trimmed to a short pointer at calendaring-jmap's own ReadTheDocs.

Closes pycalendar/calendaring-jmap#10

@tobixen

tobixen commented Sep 16, 2026

Copy link
Copy Markdown
Member

This is on my list, and I expect to have it merged to main tomorrow. Before approving this pull request, I should ensure all tests passes. I just released version 3.3.1, so at least the conflict in CHANGELOG needs some massaging.

Background: The JMAP logic was requested in the CalDAV-library based on an old user request, but it doesn't really belong in the CalDAV-library. Sashank has now been reimplementing the logic in a separate JMAP-library, and the code should be yanked out of the CalDAV-libary. To try not to break backward-compatibility, the CalDAV-library will come with an optional dependency on the JMAP-library and wrapper functions. If it's hard to maintain backward-compatibility and one has a good reason for it, then breaking changes are OK - because the JMAP-support in CalDAV 3.x was marked up as experimental in the documentation.

@tobixen

tobixen commented Sep 16, 2026

Copy link
Copy Markdown
Member

With jmap as an optional dependency, the import will break with a bare ModuleNotFoundError. I think we should wrap it around a try/except-block with descriptive error messages (install caldav[jmap] or jmap or something like that).

@SashankBhamidi

Copy link
Copy Markdown
Collaborator Author

Wrapped the calendaring_jmap import in try/except with a descriptive error pointing at caldav[jmap].

The re-export shim carried only the top-level names, so the import line
the 3.3 documentation spells out verbatim - `from caldav.jmap.error
import JMAPAuthError` - died with a ModuleNotFoundError instead of
going through the new DeprecationWarning.  calendaring-jmap mirrors the
module layout caldav.jmap used to have, so each public submodule is now
aliased into sys.modules and bound as an attribute of the package.

Prompt: look into PR [#711]

Assisted-By: Claude Opus 5 <noreply@anthropic.com>
The CHANGELOG said old imports keep working, but not that a plain `pip
install caldav` no longer gives you caldav.jmap at all, and neither it
nor jmap.rst mentioned that calendaring-jmap is AGPL-3.0-or-later where
caldav is GPL-3.0-or-later OR Apache-2.0.  Same import path, different
terms - easy to miss for anyone who picked the Apache option.  Both are
now written up as breaking changes, and the JMAP page carries a
licensing note next to the install instructions.

Prompt: look into PR [#711]

Assisted-By: Claude Opus 5 <noreply@anthropic.com>
tests/test_jmap_wrapper.py imported calendaring_jmap at module level, so
the documented developer setup - `pip install -e ".[test]"`, which does
not carry the jmap extra - ended in a collection error rather than a
skip.  Only tox installs the extra, so CI never saw it.  Resolved with
pytest.importorskip, the convention the rest of the test suite already
uses for an optional dependency, rather than by adding calendaring-jmap
to the test extra: that would put an AGPL package into every
contributor's test environment.

Prompt: look into PR [#711]

Assisted-By: Claude Opus 5 <noreply@anthropic.com>
http-libraries.rst still described the async JMAP client as the one
exception to the fallback chain, "built on niquests' AsyncSession" -
that is require_async_session() in caldav/lib/http_sync.py, which the
JMAP extraction deleted.  The choice of HTTP library is
calendaring-jmap's
own concern now, and the advice to depend on caldav[niquests] for it
was moot anyway, since calendaring-jmap requires niquests outright.

Prompt: look into PR [#711]

Assisted-By: Claude Opus 5 <noreply@anthropic.com>
required_library_error()'s only caller was require_async_session(),
which went with the JMAP extraction, and its tests pinned the exact
wording using "the async JMAP client" as the example - a string naming
something no longer in the tree, in tests that can no longer go red for
a real user.  The helper is kept, since the condition it describes
recurs whenever a component is built on a single library, but the
example is now a hypothetical.  The AsyncSession and
require_async_session removals from caldav.lib.http_sync get the
CHANGELOG line they were missing.

Prompt: look into PR [#711]

Assisted-By: Claude Opus 5 <noreply@anthropic.com>
@tobixen

tobixen commented Sep 18, 2026

Copy link
Copy Markdown
Member

I asked Claude to have a look, and it did record some "breaking changes":

  • Breaking: caldav.jmap no longer works on a plain pip install caldav. The implementation moved to the standalone calendaring-jmap package, which is an optional dependency - install caldav[jmap] (or calendaring-jmap). Importing caldav.jmap without it raises an ImportError saying so.
  • Breaking: caldav[jmap] brings dependencies caldav itself does not have. calendaring-jmap 1.1.0 requires icalendar>=7.3.0 (caldav asks only for icalendar>6.0.0, so the extra raises the floor), and it requires both niquests and requests outright - so requests is installed even in the environments that deliberately avoid it, see HTTP Library Configuration.
  • Breaking: the JMAP code is licensed differently from the rest of caldav. caldav is GPL-3.0-or-later OR Apache-2.0; calendaring-jmap is AGPL-3.0-or-later. The import path is unchanged, so this is easy to miss: if you relied on the Apache-2.0 option, note that the JMAP code you get through caldav[jmap] carries the AGPL network-copyleft obligation. Nothing changes for users of caldav without the jmap extra.

The first one is already an accepted compromise, the license thing I've also explicitly accepted, but the middle one seems a bit silly. Is there anything in the current jmap code that needs icalendar 7.3.0? If not, could you release one version with stated support for icalendar 6? It also seems silly to have both requests and niquests as hard dependencies in the project. (see https://github.com/python-caldav/caldav/blob/master/docs/source/http-libraries.rst for some information on requests vs niquests)

@tobixen

tobixen commented Sep 18, 2026

Copy link
Copy Markdown
Member

Rather than bouncing a ball, I decided to ask claude to amend the pull request. It added some few commits - and apparently broke the tests while being at it. I've looked a bit through, some of the changes it did seems sound. I'm not sure I'm sharing the concern that it's a crime to let test code depend on an AGPL-library. At least things should be squashed before merging to the main branch.

@SashankBhamidi

Copy link
Copy Markdown
Collaborator Author

@tobixen 7.3.0 isn't arbitrary. GHSA-qjcq-q7h7-r74v, the VALARM REPEAT memory/CPU exhaustion bug, is mine, I found and fixed it upstream, it's in 7.2.1. calendaring-jmap parses calendar data straight off a JMAP server, untrusted input by definition, so I'm not shipping anything below that fix.

The other reasonn is the globally-unique-TZID resolution (issue #313), the /freeassociation.sourceforge.net/Europe/Berlin style TZIDs libical and Evolution emit. calendaring-jmap's own TZID normalization depends on it directly, with a test that exercises exactly that case. That landed in 7.3.0, not 7.2.x, so that's where the floor has to sit.

6.x is off the table. Because it's missing both of those, and I'm not going to loosen a floor I set for a reason just to make the pin number smaller. Checked the changelog against what's coming up on my roadmap too, nothing past 7.3.0 is needed, so this holds for a while.

The niquests+requests thing, yeah, that one's just sloppy. _http.py is written to fall back from niquests to requests, only one should ever be a hard dependency, pyproject.toml just doesn't match that. I'll get it fixed on my side, not urgent enough to cut a release over right now though, mid-milestone and I'd rather the real fix land as part of a proper v2 than get bolted on as a patch to this PR.

I will let you know when I make the next release with this fix.

@SashankBhamidi

Copy link
Copy Markdown
Collaborator Author

That fix is merged now, main is at 514b84c, if you want to pin it before I cut a release:

calendaring-jmap @ git+https://github.com/pycalendar/calendaring-jmap.git@514b84c

Once I tag a release you can swap that for a normal version pin.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-export caldav.jmap through calendaring-jmap in python-caldav

2 participants