feat(storage): multi bucket download file api #5656
Conversation
ekjotmultani
force-pushed
the
feat/multi-bucket-download-file
branch
from
c23fc64
to
cf85cf1
Compare
|
there don't seem to be any unit tests since this api actually just further calls the downloadData api so those tests suffice |
packages/storage/amplify_storage_s3_dart/lib/src/amplify_storage_s3_dart_impl.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I don't think we need to duplicate all the test cases for the secondary bucket. we just need to tests that we can download a file usign bucket option.
There was a problem hiding this comment.
This makes sense, I'll make a few separate ones under the standard config to test this then
| @@ -216,15 +216,15 @@ class AmplifyStorageS3Dart extends StoragePluginInterface | |||
| defaultPluginOptions: const S3DownloadDataPluginOptions(), | |||
| ); | |||
|
|
|||
| final s3Options = StorageDownloadDataOptions( | |||
| options = StorageDownloadDataOptions( | |||
There was a problem hiding this comment.
it causes errors like "Local variable 'options' can't be referenced before it is declared." for using options?.bucket in the constructor, and another complaint of The final variable 'options' can't be read because it's potentially unassigned at this point.
There was a problem hiding this comment.
how come it was final before? why to need to change it?
There was a problem hiding this comment.
it was final before since it was called s3Options, different variable name than the one that is passed into the function as a parameter as well
There was a problem hiding this comment.
I see the issue here, it is overriding the function input parameter and it is wrong to do so. Why to need to make this change? I would suggest to reveret it.
There was a problem hiding this comment.
I did it to maintain consistency with the other functions options being called just options instead of s3options. I will revert it, I need to rebase with the most recent push to multi-bucket anyways
added integration tests
…ge_s3_dart_impl.dart Co-authored-by: NikaHsn <[email protected]>
added integration tests
…ge_s3_dart_impl.dart Co-authored-by: NikaHsn <[email protected]>
ekjotmultani
force-pushed
the
feat/multi-bucket-download-file
branch
from
815288a
to
324febe
Compare
Description of changes:
added integration tests
Updated download file options to accept bucket parameter
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.