[SYNPY-1875] Copy and deprecate copyWiki function - #1434
Conversation
There was a problem hiding this comment.
🔥 From a product perspective, the usage of this makes sense. I wonder if we should take this opportunity to update the parameters from "destination_" to "target_" which would mimic the unix copy, and thoughts from the team about this?
I'll defer to @BryanFauble / @danlu1 for final review. PInging Dan especially as this will be new copy functionality that exists for objects.
| root_wiki = await root_wiki.store_async(synapse_client=syn) | ||
|
|
||
| # Allow the wiki header tree to become consistent | ||
| await asyncio.sleep(5) |
There was a problem hiding this comment.
https://sagebionetworks.jira.com/browse/SYNPY-1901
If you want to take on the jira, this would be updated to use the new path. At a minimum it would be nice to have a tenacity wait for consistency instead of sleep statements for just your code here, and then applied to existing files as a part of SYNPY-1901
I would say no to this, unless we wanted to do it across the entire project. |
download_from_url is synchronous and its HTTP path blocks on requests, so calling it directly from the async wiki methods stalled the asyncio event loop for the duration of the transfer. Route the four call sites through loop.run_in_executor with the client thread pool, matching the pattern already used in download_by_file_handle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… debug Every log statement in wiki.py now carries an owner/wiki ID prefix so messages can be attributed to a specific wiki page. The per-page copy chatter is demoted from info to debug, leaving info for the higher level operations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both sub page ID inputs are typed str | None in copy_async, so a float can only arrive from a caller ignoring the type hints. The float branch was speculative parity with the legacy copyWiki, which called int() on an untyped argument. Int is still accepted since wiki page IDs are numeric. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When copying a sub-tree, Synapse returns the owner's entire flat header list and everything outside the requested branch is discarded silently. The skipped page IDs are now logged once in copy_async, after the recursive collection has finished, so a missing page is traceable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…unds Add TODO comments at the four sites in wiki.py that push the synchronous download_from_url onto a worker thread, pointing at the ticket tracking the async httpx-based replacement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Problem:
synapseutils.copyWiki()was the only way to copy a wiki tree from one entity toanother, and it is legacy code that sits outside the object-oriented model layer:
synclient as its firstpositional argument, so it cannot be used from an async context and does not follow
the
synapse_clientkeyword pattern used everywhere else in the new API.entitySubPageId,destinationSubPageId,updateLinks,entityMap) that are inconsistent with the rest of the OOP models.WikiPagemodel, so users who have adoptedsynapseclient.modelshad to drop back down tosynapseutilsto copy a wiki —the same gap that was closed for
DownloadList,sync_to_synapse/sync_from_synapse, storage locations, and migration in earlier work.This is the
copyWikihalf of the ongoing migration ofsynapseutilsbulkoperations onto the object-oriented models (SYNPY-1875).
Solution:
Add
WikiPage.copy_async()(and its generated syncWikiPage.copy()) as theobject-oriented replacement for
copyWiki(), and deprecate the legacy function.New API function —
synapseclient/api/file_services.pyAdded
post_file_handles_copy(), wrappingPOST /filehandles/copy, used to copywiki attachment file handles to the destination. It batches requests by
MAX_FILE_HANDLE_PER_COPY_REQUESTinternally so callers do not have to, and it isre-exported from
synapseclient.api. This puts the file-handle-copy REST call in theapi/layer where the model can reuse it, rather than reimplementing the batchingthat
synapseutils.copyFileHandles()does.Deprecation —
synapseutils/copy_functions.pycopyWiki()is marked@deprecated(version="5.0.0")for removal in 6.0.0, with adocstring migration example pointing at
WikiPage.copy/WikiPage.copy_async. Thefunction itself is unchanged, so existing callers keep working.
Supporting changes
synapseclient/models/protocols/wikipage_protocol.py— synccopy()signatureadded so IDE type hints resolve.
docs/reference/experimental/sync/wiki.mdanddocs/reference/experimental/async/wiki.md—copy/copy_asyncadded to theWikiPagemembers lists so mkdocstrings generates the anchors and[synapseclient.models.WikiPage.copy]cross-references resolve.The orchestration is split into small module-level helpers
(
_validate_and_format_copy_inputs,_copy_wiki_pages,_collect_wiki_sub_tree_headers,_update_internal_links,_update_synapse_id_references,_ensure_destination_has_no_root_wiki,_get_existing_destination_wiki_page) so each piece is unit-testable withoutstanding up a full wiki tree.
Testing:
Unit tests —
tests/unit/synapseclient/models/async/unit_test_wiki_async.pyAdded a
TestWikiPageCopyclass coveringcopy_asyncend to end with mocks, plus atest class per helper. Coverage includes:
owner_id, invaliddestination_owner_id, invalidentity_mapkeys and values, non-numeric sub-page IDs.entity_sub_page_idraisesValueError, sub-page ID given for a source with nowiki raises
ValueError, sub-page ID type coercion.destination_sub_page_id, nonexistentdestination_sub_page_id, and errorpropagation from the destination-page and header-tree lookups.
update_linksonly (the default),entity_maponly, both, and neither.
non-mutation of the input headers, link and Synapse-ID rewriting across every
copied page,
Nonemarkdown becoming an empty string, and hierarchy preservationin
_copy_wiki_pagesincluding the create-under and write-into destination cases.Integration tests —
tests/integration/synapseclient/models/async/test_wiki_async.pyAdded a
TestWikiPageCopyclass that copies against a real Synapse backend:test_copy_entire_wiki_tree— full tree copy, verifying hierarchy, markdown,attachments, and rewritten internal links.
test_copy_wiki_sub_tree— copying only the sub-tree rooted at a given page.test_copy_wiki_into_existing_destination_page— copying into an existingdestination wiki page via
destination_sub_page_id.test_copy_wiki_from_entity_without_wiki— source with no wiki returns an emptylist.