refactor download interfaces to treat immutable files and mutable versions more uniformly #993
Labels
No Label
0.2.0
0.3.0
0.4.0
0.5.0
0.5.1
0.6.0
0.6.1
0.7.0
0.8.0
0.9.0
1.0.0
1.1.0
1.10.0
1.10.1
1.10.2
1.10a2
1.11.0
1.12.0
1.12.1
1.13.0
1.14.0
1.15.0
1.15.1
1.2.0
1.3.0
1.4.1
1.5.0
1.6.0
1.6.1
1.7.0
1.7.1
1.7β
1.8.0
1.8.1
1.8.2
1.8.3
1.8β
1.9.0
1.9.0-s3branch
1.9.0a1
1.9.0a2
1.9.0b1
1.9.1
1.9.2
1.9.2a1
LeastAuthority.com automation
blocker
cannot reproduce
cloud-branch
code
code-dirnodes
code-encoding
code-frontend
code-frontend-cli
code-frontend-ftp-sftp
code-frontend-magic-folder
code-frontend-web
code-mutable
code-network
code-nodeadmin
code-peerselection
code-storage
contrib
critical
defect
dev-infrastructure
documentation
duplicate
enhancement
fixed
invalid
major
minor
n/a
normal
operational
packaging
somebody else's problem
supercritical
task
trivial
unknown
was already fixed
website
wontfix
worksforme
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Reference: tahoe-lafs/trac-2024-07-25#993
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Currently, downloading a mutable file is quite complicated unless the download_best_version() method of IMutableFileNode is used. This has several disadvantages:
The attachment gives some proposed interface changes worked out with Brian on IRC. Among other improvements these would allow calling
get_best_readable_version()
on any filenode, and then usingread
to download to a consumer. For the time being mutable files would still be downloaded in a single lump, but clients using these new interfaces would not have to change when we fix that.('diff' got confused and started interleaving changes to unrelated methods, so compare the attachment with source:src/allmydata/interfaces.py.)
Attachment interface-changes.py (21646 bytes) added
Proposed refactoring of file download interfaces
Too disruptive / not enough time left to do this for 1.7.
Brian: is your new-downloader patch predicated on this patch? If so then we either need to bite the bullet and accept this disruptive refactoring for 1.7, at the risk of either destabilizing or delaying 1.7 release, or else plan to release 1.7 without new-downloader.
(David-Sarah and Brian and I discussed the new-downloader in v1.7 or not on IRC. We agreed that it might be good to plan v1.7 with no new-downloader and that it might be good to delay v1.7 by a few weeks to have a solid, reliable new-downloader.)
Note that the patch attached to this ticket is only a proposed API, not an implementation of it.
[Here]source:src/allmydata/frontends/sftpd.py@4521#L706 is an example of code that forks depending on whether it's downloading a mutable or immutable file, in the SFTP frontend:
With the proposed interface it could be:
Since the new downloader is milestone 1.8, this can wait until after 1.7.1, though.
Kevan is working on this in the patches for #393 (MDMF). Some comments on those patches relating to this ticket:
Yes, it can be, and should take an
IMutableUploadable
rather than a bytestring.The second paragraph here could be misread; perhaps change it to
How can we move this ticket out of
review-needed
state? I know! Kevan could review it and we could apply the patch to trunk! Kevan: interested? If we're not going to "apply the patch to trunk" because it is only interface changes and not implementation then let's remove thereview-needed
tag until we have an implementation.An implementation of this ticket (including the interface) is in #393, so I don't think there's anything to apply here. I'll remove review-needed. I'm tempted to say that #993 could be marked as a duplicate of #393, but it might be useful to be able to discuss the interface independently of #393 as a whole, which is fairly long and involves other issues.
(Maybe we should make a trac keyword that says that a ticket is in particular need of design feedback or review but doesn't involve any code changes. Then 'review-needed' could be used only for code changes needing a review before they can be committed, and the other keyword could be used for tickets like #993, which don't have a patch that we'd want to apply but could benefit from other perspectives.)
This is still addressed by #393, so it should stay in the 1.9.0 milestone.
In /tahoe-lafs/trac-2024-07-25/issues/5475#comment:76218, zooko said:
This affects the new
IMutableFileversion
interface ininterfaces.py
: it should either grow aget_roothash()
method, orget_sequence_number()
should be changed toget_sequence_number_and_roothash()
(or some more concise name) that returns a pair.(The object represents a particular version, so a single method returning a pair is not necessary to avoid race conditions.)
not making it into 1.9
The main changes suggested by this ticket were part of #393, and are present in 1.9. (comment:76225 isn't implemented, but that isn't critical.)
Don't know why the Version field changed.
does that mean we can close this?
davidsarah said yes. Yay!