Currently when performing a file migration from public:// or private:// files are stored as s3:///path/to/file.
Unless a metadata reload is done after the migration S3FS will not be aware of the files natively.
Perform a file migration from local files.
Immediately after copying the file perform a database update to strip the prefix and replace with scheme://
or
Manually upload the file and inject the data into files table.
None
S3fsFileMigrationBatchinterface::copyOperation changed to require an array $config as its first parameter, and the context passed reference is now type hinted as an array.
- public static function copyOperation($config, array $file_paths, $total, $source_folder, $target_folder, $scheme, &$context) {
+ public static function copyOperation(array $config, array $file_paths, $total, $source_folder, $target_folder, $scheme, array &$context) {Imported files are correctly stored as public:// or private:// in the s3fs_file table instead of as s3://schemeprefix/
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff-3202909-3-7.txt | 2.78 KB | cmlara |
| #7 | file_migration_does-3202909-7.patch | 8.23 KB | cmlara |
| #3 | file_migration_does-3202909-3.patch | 8.19 KB | cmlara |
Comments
Comment #2
cmlaraThis is an initial go at the patch.
This does add an API change to S3fsFileMigrationBatch:copyOperation() requiring config as a first object.
This is so we can create our own client which is required
Because we are pulling out copy and using putobject() we need to check for the 255 character limit and for errors in putObject().
Returning files with issues so the admin will know what was not uploaded.
This has a todo of the 5GB limit once that is figured out in #3204634: 5GB limit when saving file. but that will not be a blocker for this as this patch is retaining the existing limitation.
Comment #3
cmlaraWith the actual patch file
Comment #4
darvanenI must be missing something here. This looks like a much better implementation of batch processing, but where is it updating the database, unless this is it?
Comment #5
cmlaraCorrect
$streamWrapper->writeUriToCache($uri);is where we update the database 's3fs_files' "cache table"It takes the uri it is given (ex 'public://test.txt') checks the object exists in the bucket, gets its metadata and then writes it into the s3fs_files table.
Honestly this function feels like it doesn't belong in S3FsStreamWrapper however that is where it currently is.
I actually had a side commit where I had started yanking it (and other) functions out and putting them in S3fsService but stopped so I could bring it up as separate issue since it would involve an API break, writeUriToCache is specifically called out to be used by other modules in its docblock.
Comment #6
darvanenWell, in that case I see no issues with this patch.
I agree the caching methods should probably go in the service, or even a new S3fsCacheService.
Comment #7
cmlaraUpdating the issue description to indicate the API change for future reference and to indicate the storage change.
Changing a README section for slightly clearer language, and most importantly making sure I don't make the same mistake I made in #3205964: S3fsService:writeFolders is not compatible with interface where the $config interface deceleration didn't match the function implementation. At the same time I had re-checked the new private function updateProgress() and noted no typehint had been applied previously because $context wasn't type hinted earlier so applying that now so it can be correctly type hinted.
In addition while glancing through older issues I did stumble on #3145478: s3fs-refresh-cache mistakenly assumes public/private takeover is selected from 7.x where the Cache Refresh instead will intentional sync files as s3://pathprefix/file when takeover is not enabled while this patch takes an opposite approach and is attempting to make sure the database always uses public:// or private:// instead of s3://prefix/. Personally I believe this is acceptable and is the more appropriate approach however its worth acknowledging that this would be a difference between the 7.x-3.x branch and the 8.x-3.x branch. For those who need to access files without takeover enabled the prefix paths can be changed to a nonexistent key space (random long string) and the cached refreshed. Its essentially a question of "is the path dedicated to public:// and private:// a reserved key space prefix that should not be used in any manner expect with the public:// and private:// StreamWrappers"
Comment #9
cmlaraCommitted (with todo note removed)