Page MenuHomePhabricator

Move Conduit file upload logic into a separate class
ClosedPublic

Authored by epriestley on May 26 2015, 3:18 PM.
Tags
None
Referenced Files
F15198978: D13016.diff
Sun, Feb 23, 5:55 AM
Unknown Object (File)
Fri, Feb 21, 3:54 PM
Unknown Object (File)
Thu, Feb 20, 2:05 AM
Unknown Object (File)
Fri, Feb 14, 6:33 PM
Unknown Object (File)
Tue, Feb 11, 12:25 PM
Unknown Object (File)
Tue, Feb 11, 2:22 AM
Unknown Object (File)
Sat, Feb 8, 4:53 AM
Unknown Object (File)
Sat, Feb 8, 4:53 AM

Details

Summary

Ref T8259. Currently, arc upload uses new logic but arc diff uses older logic internally. This prevents arc diff from uploading files larger than 4MB to newer servers.

Split the upload logic apart so the two upload pathways can share it. Callers now build a list of FileDataRefs and hand them to an Uploader for upload.

Test Plan

Ran arc upload on:

  • One file.
  • Several files.
  • Small files.
  • Big files.
  • Directories.
  • Unreadable files.
  • Files full of random data.
  • The same file over and over again.
  • The same big file over and over again.
  • Artificially broke file.allocate and redid some of the simple cases (large/chunked aren't expected to work in this case).

Diff Detail

Repository
rARC Arcanist
Branch
upload1
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/upload/ArcanistFileUploader.php:215XHP16TODO Comment
Advicesrc/workflow/ArcanistUploadWorkflow.php:72XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 6284
Build 6306: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Move Conduit file upload logic into a separate class.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
joshuaspence added inline comments.
src/upload/ArcanistFileDataRef.php
116–126

You could almost use PhutilInvalidStateException here. In any case, I don't think that setData() or setPath() are translatable.

btrahan edited edge metadata.
btrahan added inline comments.
src/upload/ArcanistFileDataRef.php
55

array typehint maybs? i'm not quite sure how one would use this

This revision is now accepted and ready to land.May 27 2015, 5:03 PM
src/upload/ArcanistFileDataRef.php
55

This is just a blob of file data (e.g., from Filesystem::readFile()). I'll put a third diff in this sequence to finish the docs.

src/upload/ArcanistFileDataRef.php
55

Thanks!

This revision was automatically updated to reflect the committed changes.