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

Refactoring of S3AccessIO driver #5072

Closed
poikilotherm opened this issue Sep 20, 2018 · 9 comments
Closed

Refactoring of S3AccessIO driver #5072

poikilotherm opened this issue Sep 20, 2018 · 9 comments

Comments

@poikilotherm
Copy link
Contributor

poikilotherm commented Sep 20, 2018

Hi @landreev

while digging into the driver to get things running for #4690 and #5068 it seems you wrote most of the storage driver for AWS S3. (At least you opened #3921 😉 )

In #5059 I started work to implement unit and integration testing for the driver. Are you interested to join?

@pdurbin
Copy link
Member

pdurbin commented Sep 21, 2018

@poikilotherm I see you're already working away on this over at poikilotherm@5251045 adding unit tests. Great! Thanks!

@poikilotherm
Copy link
Contributor Author

Yes I am - but it is quite an amount of work... And many details of the implementation are unknown to me.

Example: in getMainFileKey() it is not obvious to me, why this method is not written sideeffect free. I hoped that @landreev could help me out with a lots of details about the driver... 😉

@poikilotherm
Copy link
Contributor Author

Maybe @ferrys, @matthew-a-dunlap, @bsilverstein, @oscardssmith and other can look into this, too?

@oscardssmith
Copy link
Contributor

Most of the changes I made were making StorageIO parametric on the type of DVObject stored. My sense in general is that this area was rather underdesigned, and the reason for most of these types of questions is because "It worked and we want to move on". What is the side effect by the way?

@poikilotherm
Copy link
Contributor Author

What is the side effect by the way?

Both the instance vars key and bucketName get set inside, although key is set again with the return value of the function. This could be by design or unintended - hard to predict without comments and not being involved before... 😉

@poikilotherm
Copy link
Contributor Author

Mentioning #5165

@poikilotherm
Copy link
Contributor Author

Mentioning #5996 here...

@pdurbin
Copy link
Member

pdurbin commented Nov 16, 2023

We now have some tests for S3AccessIO:

@cmbz
Copy link

cmbz commented Aug 20, 2024

To focus on the most important features and bugs, we are closing issues created before 2020 (version 5.0) that are not new feature requests with the label 'Type: Feature'.

If you created this issue and you feel the team should revisit this decision, please reopen the issue and leave a comment.

@cmbz cmbz closed this as completed Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants