Stalled: deprecate calendar_multiget in favour of multiget#672
Open
Stalled: deprecate calendar_multiget in favour of multiget#672
Conversation
* A trivial todo was fixed (I'm not so good at ReST and didn't have time checking up how to do internal links - now I asked Claude to fix it. Claude improved the language in the relevant sentence while being at it) * "3.0" was replaced with "3.x" consistently. 3.2 is out, but the doc remains relevant. * The section on deprecated things added "will be removed in 4.0" in the section header. That's or one thing incorrect (many things will be removed the earliest in 5.0) and or the other thing it doesn't need to be included in the header. prompt: The v3-migration page has a todo-comment - please fix followup=prompt: Check if there are other todo-markers under docs/source that can be dealt with easily followup-prompt: commit
…sync client When using niquests in async mode, digest auth must use AsyncHTTPDigestAuth. The sync HTTPDigestAuth.handle_401() calls r.connection.send() which returns a coroutine in async context, causing "AttributeError: 'coroutine' object has no attribute 'history'". Reported at jawah/niquests#387 (comment) prompt: Please check jawah/niquests#387 (comment), the last comment hints that the problem is in the caldav library AI Prompts: claude-sonnet-4-6: Please check jawah/niquests#387 (comment), the last comment hints that the problem is in the caldav library
Add fixtures and helpers that subsequent phases depend on: - async_calendar2: second test calendar fixture for tests needing two distinct calendars (testLoadEvent, testCopyEvent equivalents) - async_journal_list: VJOURNAL calendar fixture for journal tests - _make_async_client_with_params(): helper to build a fresh async client from the server config with selected kwargs overridden (needed by wrong-password / wrong-auth-type tests) - Import ev1_static, ev2_static, ev3_static, evr_static, evr2_static, todo_static, todo2_static, todo3_static, journal_static from test_caldav so old-date search tests can reuse the same iCalendar fixtures without duplication (aliases avoid name clashes with the existing near-future ev1()/ev2()/todo2() generator functions) prompt: An issue was created with `git bug bug new`, it has id 099db26 comment has id 009999d. Please start with Phase 1 and update the issue, but in a separate temporary worktree on a branch spun off from v3.2-preparations Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> AI Prompts: claude-sonnet-4-6: please help me investigate the tests/test_async_integration.py::TestAsyncForZimbra::test_lookup_event breakage. It's a known problem with zimbra that a GET towards an event URL frequently yields 404, but the .load()-method should have fallbacks to .multiget(), and the caldav-server-tester in ~/caldav-server-tester claims that the current versions of Zimbra have no problems with such GETs claude-sonnet-4-6: wait. save-load.get-by-url actually works for zimpra, the problem is "duplicated UIDs across calendars"? We should probably sharpen the compatbility hints and checks in ~/caldav-server-tester, as well as sharpen the test code to avoid such problems. claude-sonnet-4-6: continue claude-sonnet-4-6: pehaps save-load.url.at-sign ? claude-sonnet-4-6: ok, let it be url.at-sign claude-sonnet-4-6: Some changes were lost due to a `git reset`. Please reintroduce the change in caldav/compatibility_hints.py and caldav/base_client.py. Probably the other changes didn't have any value.
Add 7 async equivalents of the sync RepeatedFunctionalTestsBaseClass CRUD tests: - test_get_supported_components: verify get_supported_components() includes VEVENT - test_lookup_event: add_event(), get_event_by_uid(), Event(url=…).load(), NotFoundError on missing UID - test_create_overwrite_delete_event: no_create/no_overwrite flag semantics, same-UID overwrite, delete and 404 verification - test_object_by_uid: add_todo with known UID, get_object_by_uid(), exact match only (prefix and suffix must not match) - test_load_event: add_event() returns usable object; load() populates data on both the returned handle and a freshly fetched one (uses async_calendar2) - test_copy_event: copy() within same calendar (new UID), cross-calendar copy with keep_uid, same-calendar keep_uid overwrite (uses async_calendar2) - test_multi_get: calendar_multiget() with two URLs, load_by_multiget() All tests use add_event/add_todo (not the deprecated save_event/save_todo), await every coroutine, and gate on is_supported() feature flags. prompt: continue with phase 2 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add 'save-load.mutable' to the FEATURES dict in compatibility_hints.py:
it indicates that a calendar object resource can be modified and PUT
back to the server (the server must reflect the change on the next
GET/REPORT). Defaults to 'full' as virtually all servers support this;
the check exists to surface rare exceptions like Google Calendar's
legacy CalDAV API.
Replace every use of the old 'no_overwrite' quirk flag in test_caldav.py
with the new feature:
skip_on_compatibility_flag("no_overwrite") → skip_unless_support("save-load.mutable")
if not check_compatibility_flag("no_overwrite"): → if is_supported("save-load.mutable"):
prompt: Create tests for a new feature checking if it's possible to
overwrite a calendar event with new data. Should replace the old
"overwrite" compatibility flag. (Perhaps this "incompatibility" was due
to the caldav library not supporting etags and not updating from no
sequence to SEQUENCE:1 ... i somehow doubt we'll find any servers not
supporting this feature, but we should have the check anyway).
load-save.mutable ? Update the compatibility_hints.py in
/tmp/caldav-async-tests/ and update the tests there as well.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- multiget() now dispatches to _async_multiget_objects() for async clients, following the ASYNC_DUAL_MODE pattern - extracted _post_multiget() shared post-processing helper - added _async_multiget_objects() async counterpart - _post_load_by_multiget() now calls iter() so it works with both generator (_multiget) and list (_async_multiget) inputs - test_multi_get updated to use multiget() directly (not deprecated calendar_multiget) - fix test_async_integration.py: use save-load.mutable instead of no-overwrite flag prompt: tests/test_async_integration.py::TestAsyncForXandikos::test_multi_get fails in the git worktree branch on /tmp/caldav-async-tests/, fix multiget in collection.py to be async-aware, following the pattern in docs/design/ASYNC_DUAL_MODE.md followup-prompt: wait. calendar_multiget is deprecated and scheduled for deletion. Any test code exercising calendar_multiget can be removed or rewritten to use self.multiget instead. followup-prompt: commit (include the save-load.mutable fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> AI Prompts: claude-sonnet-4-6: commit 3dff59f3 includes a new deprecation warning, this again breaks test code. I came to think that new deprecation-warnings should probably only be introduced on major versions. Please extract the deprecation warning in a fixup (and reword the commit message). Create a new pull request with the only the deprecation warning. Apply the 4.0-label to the pull request, the title should be prefixed with "Stalled:", adn the pull request description should say that it's slated for 4.0 claude-sonnet-4-6: it's milestone, not label. And it may be v4
... but the async test still fails
…isplayname is unsupported Zimbra uses the DisplayName from the MKCALENDAR request body as the calendar URL/path, ignoring the actual request URL. When a display name is present but the calendar is being created at a UUID-derived path, Zimbra silently creates the calendar at the display-name URL instead and makes the UUID path an alias. That alias accepts PUT but returns 404 on direct GET for individual resources, so event_by_url fails for all calendars created this way. The compatibility flag `create-calendar.set-displayname: unsupported` already existed for Zimbra. The code was honouring it for the post-creation PROPPATCH but not for the MKCALENDAR body itself. This commit also applies it there: when the flag is unsupported, no DisplayName element is included in the MKCALENDAR body, so Zimbra uses the request path as the calendar identifier and PUT/GET are consistent. The post-creation PROPPATCH in both sync and async paths is now conditioned on `display_name` being set (previously `name`), so it is skipped correctly when the display name was omitted from the MKCALENDAR body. prompt: async test_lookup_event fails for zimbra but sync testLookupEvent passes, investigate why. followup-prompt: ok (implement the fix) AI Prompts: claude-sonnet-4-6: With `pytest -k 'zimbra and lookup'` the sync test passes, while the async test fails. Why? There shouldn't be any difference there? (you've already investigated two hypotheses: "zimbra does not support quoted at-sign in the URL" and "the uid has been used on another calendar, therefore the GET returns 404", both hypotheses are wrong, and does not explain the asymmetry between sync and async) claude-sonnet-4-6: retry claude-sonnet-4-6: retry claude-sonnet-4-6: retry claude-sonnet-4-6: retry claude-sonnet-4-6: ok
the unique_calendar_ids old compatibility flag seems not to be needed anymore by any servers I test.
calendar_multiget() now emits a DeprecationWarning directing callers to use multiget() instead. Slated for removal in v4.0. prompt: extract the deprecation warning from the multiget async-aware commit into a separate stalled PR for v4.0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is slated for v4.0 and will not be merged before then.
calendar_multiget()is a legacy alias formultiget(). This adds aDeprecationWarningso callers have advance notice before the method isremoved in v4.0.
Deprecation warnings are deferred to a major release to avoid breaking
test suites that use
filterwarnings("error").