Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update aws sdk + s3 improvements #4410

Merged
merged 13 commits into from
Sep 18, 2017
Merged

Update aws sdk + s3 improvements #4410

merged 13 commits into from
Sep 18, 2017

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Apr 20, 2017

  • requires: add aws sdk 3rdparty#51

  • move ask sdk from files_external/3rdparty to 3rdparty

  • update aws sdk

  • adjust files_external s3 and object store s3 to work with the updated sdk

  • improve performance for s3 external storage

external storage upload performance comparison: blackfire #

@icewind1991 icewind1991 added the 2. developing Work in progress label Apr 20, 2017
@icewind1991 icewind1991 added this to the Nextcloud 12.0 milestone Apr 20, 2017
@Xuanwo
Copy link
Contributor

Xuanwo commented Apr 25, 2017

As we talked in #3910 , is there any plan to build a new app and move aws sdk into it, so that we can support more object storage backend?

@icewind1991
Copy link
Member Author

You can already create an app to add support for an object store backend, simply have the app define the class implementing the backend.

Since the object store config specifies the class to use directly there is no need to do any further "hooking up" of things

@codecov
Copy link

codecov bot commented Jun 7, 2017

Codecov Report

Merging #4410 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #4410      +/-   ##
============================================
+ Coverage     53.36%   53.38%   +0.01%     
+ Complexity    22576    22535      -41     
============================================
  Files          1409     1410       +1     
  Lines         87338    87229     -109     
  Branches       1340     1340              
============================================
- Hits          46611    46565      -46     
+ Misses        40727    40664      -63
Impacted Files Coverage Δ Complexity Δ
lib/private/RichObjectStrings/Validator.php 78.26% <0%> (-3.89%) 13% <0%> (ø)
lib/private/AppFramework/Http/Request.php 90.86% <0%> (-1.82%) 96% <0%> (-52%)
lib/private/Group/Group.php 87.62% <0%> (-1.27%) 51% <0%> (-1%)
apps/files_external/lib/Lib/Storage/AmazonS3.php 0% <0%> (ø) 110% <0%> (+18%) ⬆️
lib/private/legacy/template/functions.php 11.11% <0%> (ø) 0% <0%> (ø) ⬇️
...te/Authentication/LoginCredentials/Credentials.php 100% <0%> (ø) 2% <0%> (-2%) ⬇️
...s/files_external/lib/Lib/Storage/StreamWrapper.php 0% <0%> (ø) 14% <0%> (-5%) ⬇️
lib/private/Files/ObjectStore/S3ObjectTrait.php 85.71% <0%> (ø) 3% <0%> (?)
lib/private/Security/IdentityProof/Signer.php 80.64% <0%> (+0.64%) 7% <0%> (ø) ⬇️
lib/private/Files/ObjectStore/S3.php 50% <0%> (+50%) 2% <0%> (-3%) ⬇️
... and 1 more

@icewind1991 icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 7, 2017
@icewind1991 icewind1991 changed the title [wip] Update aws sdk Update aws sdk + s3 improvements Jun 7, 2017
@icewind1991
Copy link
Member Author

This is ready for review imo

@nickvergessen
Copy link
Member

hmm couldnt the deps be in the files_externals composer instead of the global one?

@icewind1991
Copy link
Member Author

I moved them to the global one since it is also used by the object store backend which is in core

@MorrisJobke
Copy link
Member

@icewind1991 Could you please fix the updated 3rdparty

@icewind1991
Copy link
Member Author

rebased

@MorrisJobke
Copy link
Member

I resolved the conflict and updated the 3rdparty module.

@MorrisJobke
Copy link
Member

I get this when I run the installation:

{
    "app": "files_external",
    "level": 3,
    "message": {
    	"Exception":"Aws\\S3\\Exception\\S3Exception",
    	"Message":"Error executing \"CreateBucket\" on \"http://abc.127.0.0.1:4567/\"; AWS HTTP error: cURL error 6: Couldn't resolve host name",
    	"Code":0,
    	"Trace":"
    		#0 3rdparty/aws/aws-sdk-php/src/WrappedHttpHandler.php(97): Aws\\WrappedHttpHandler->parseError(Array, Object(GuzzleHttp\\Psr7\\Request), Object(Aws\\Command), Array)
    		#1 3rdparty/guzzlehttp/promises/src/Promise.php(203): Aws\\WrappedHttpHandler->Aws\\{closure}(Array)
    		#2 3rdparty/guzzlehttp/promises/src/Promise.php(174): GuzzleHttp\\Promise\\Promise::callHandler(2, Array, Array)
    		#3 3rdparty/guzzlehttp/promises/src/RejectedPromise.php(40): GuzzleHttp\\Promise\\Promise::GuzzleHttp\\Promise\\{closure}(Array)
    		#4 3rdparty/guzzlehttp/promises/src/TaskQueue.php(47): GuzzleHttp\\Promise\\RejectedPromise::GuzzleHttp\\Promise\\{closure}()
    		#5 3rdparty/guzzlehttp/promises/src/Promise.php(234): GuzzleHttp\\Promise\\TaskQueue->run()
    		#6 3rdparty/guzzlehttp/promises/src/Promise.php(267): GuzzleHttp\\Promise\\Promise->waitIfPending()
    		#7 3rdparty/guzzlehttp/promises/src/Promise.php(225): GuzzleHttp\\Promise\\Promise->invokeWaitList()
    		#8 3rdparty/guzzlehttp/promises/src/Promise.php(267): GuzzleHttp\\Promise\\Promise->waitIfPending()
    		#9 3rdparty/guzzlehttp/promises/src/Promise.php(225): GuzzleHttp\\Promise\\Promise->invokeWaitList()
    		#10 3rdparty/guzzlehttp/promises/src/Promise.php(267): GuzzleHttp\\Promise\\Promise->waitIfPending()
    		#11 3rdparty/guzzlehttp/promises/src/Promise.php(225): GuzzleHttp\\Promise\\Promise->invokeWaitList()
    		#12 3rdparty/guzzlehttp/promises/src/Promise.php(62): GuzzleHttp\\Promise\\Promise->waitIfPending()
    		#13 3rdparty/aws/aws-sdk-php/src/AwsClientTrait.php(59): GuzzleHttp\\Promise\\Promise->wait()
    		#14 3rdparty/aws/aws-sdk-php/src/AwsClientTrait.php(78): Aws\\AwsClient->execute(Object(Aws\\Command))
    		#15 lib/private/Files/ObjectStore/S3ConnectionTrait.php(102): Aws\\AwsClient->__call('createBucket', Array)
    		#16 lib/private/Files/ObjectStore/S3ObjectTrait.php(63): OC\\Files\\ObjectStore\\S3->getConnection()
    		#17 lib/private/Files/ObjectStore/ObjectStoreStorage.php(367): OC\\Files\\ObjectStore\\S3->writeObject('urn:oid:4', Resource id #778)
    		#18 lib/private/Files/Storage/Wrapper/Wrapper.php(350): OC\\Files\\ObjectStore\\ObjectStoreStorage->touch('files/welcome.t...', 1501588075)
    		#19 lib/private/Files/Storage/Wrapper/Availability.php(375): OC\\Files\\Storage\\Wrapper\\Wrapper->touch('files/welcome.t...', NULL)
    		#20 lib/private/Files/Storage/Wrapper/Wrapper.php(350): OC\\Files\\Storage\\Wrapper\\Availability->touch('files/welcome.t...', NULL)
    		#21 lib/private/Files/View.php(1127): OC\\Files\\Storage\\Wrapper\\Wrapper->touch('files/welcome.t...')
    		#22 lib/private/Files/View.php(550): OC\\Files\\View->basicOperation('touch', '/admin/files/we...', Array, NULL)
    		#23 lib/private/Files/Node/Folder.php(181): OC\\Files\\View->touch('/admin/files/we...')
    		#24 lib/private/legacy/util.php(425): OC\\Files\\Node\\Folder->newFile('welcome.txt')
    		#25 lib/private/legacy/util.php(395): OC_Util::copyr('/srv/projects/s...', Object(OC\\Files\\Node\\Folder))
    		#26 lib/private/User/Session.php(486): OC_Util::copySkeleton('admin', Object(OC\\Files\\Node\\Folder))
    		#27 lib/private/User/Session.php(361): OC\\User\\Session->prepareUserLogin(true)
    		#28 lib/private/User/Session.php(547): OC\\User\\Session->completeLogin(*** sensitive parameters replaced ***)
    		#29 lib/private/User/Session.php(326): OC\\User\\Session->loginWithPassword(*** sensitive parameters replaced ***)
    		#30 lib/private/Setup.php(406): OC\\User\\Session->login(*** sensitive parameters replaced ***)
    		#31 core/Command/Maintenance/Install.php(92): OC\\Setup->install(Array)
    		#32 3rdparty/symfony/console/Command/Command.php(256): OC\\Core\\Command\\Maintenance\\Install->execute(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
    		#33 3rdparty/symfony/console/Application.php(818): Symfony\\Component\\Console\\Command\\Command->run(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
    		#34 3rdparty/symfony/console/Application.php(186): Symfony\\Component\\Console\\Application->doRunCommand(Object(OC\\Core\\Command\\Maintenance\\Install), Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
    		#35 3rdparty/symfony/console/Application.php(117): Symfony\\Component\\Console\\Application->doRun(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
    		#36 lib/private/Console/Application.php(170): Symfony\\Component\\Console\\Application->run(Object(Symfony\\Component\\Console\\Input\\ArgvInput), Object(Symfony\\Component\\Console\\Output\\ConsoleOutput))
    		#37 console.php(92): OC\\Console\\Application->run()
    		#38 occ(11): require_once('/srv/projects/s...')
    		#39 {main}",
    	"File":"3rdparty/aws/aws-sdk-php/src/WrappedHttpHandler.php",
    	"Line":192
    },
    "method": "--",
    "remoteAddr": "",
    "reqId": "6L06558ZNHfIqjOZYMY9",
    "time": "2017-08-01T11:47:57+00:00",
    "url": "--",
    "user": "admin",
    "userAgent": "--",
    "version": "13.0.0.2"
}

This is the config:

'objectstore' =>
		array (
			'class' => 'OC\\Files\\ObjectStore\\S3',
			'arguments' =>
				array (
					'bucket' => 'abc',
					'key' => '123',
					'secret' => 'abc',
					'hostname' => '127.0.0.1',
					'port' => '4567',
					'use_ssl' => false,
					'use_path_style' => 'true',
				),
		),

And I'm using fakeS3

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 1, 2017
@icewind1991
Copy link
Member Author

@MorrisJobke should be fixed

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 18, 2017
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And please also update the autoloader then this can be merged.

@@ -394,7 +456,7 @@ public function touch($path, $mtime = null) {
$fileType = $this->filetype($path);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 453: phan says:

apps/files_external/lib/Lib/Storage/AmazonS3.php:453 PhanUndeclaredClassConstant Reference to constant RFC1123 from undeclared class \Aws\Common\Enum\DateFormat

@MorrisJobke MorrisJobke added 2. developing Work in progress 3. to review Waiting for reviews and removed 4. to release Ready to be released and/or waiting for tests to finish 2. developing Work in progress labels Sep 18, 2017
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 merged commit 1995c69 into master Sep 18, 2017
@icewind1991 icewind1991 deleted the update-aws-sdk branch September 18, 2017 15:47
@karlitschek
Copy link
Member

Can we backport this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants