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

Add NIO write operations #5

Merged
merged 13 commits into from
Feb 23, 2023

Conversation

oeph
Copy link
Contributor

@oeph oeph commented Feb 8, 2023

Ticket: #4

Description of changes:

  • This adds the NIO FileSystemProvider methods as well as a implementation of a writable channel
  • With this pr the async s3 client is using now the s3 transfer manager and therefore the crt builder

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@oeph oeph changed the title [WIP] Add NIO write operations Add NIO write operations Feb 14, 2023
@oeph
Copy link
Contributor Author

oeph commented Feb 14, 2023

@markjschreiber could you have a look at this?

@@ -260,11 +260,8 @@ private S3AsyncClient asyncClientForRegion(String regionString){
Region region = regionString.equals("") ? Region.US_EAST_1 : Region.of(regionString);
logger.debug("bucket region is: '{}'", region.id());

return S3AsyncClient.builder()
return S3AsyncClient.crtBuilder()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice update

Copy link
Contributor

@markjschreiber markjschreiber left a comment

Choose a reason for hiding this comment

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

Looking good. Some improvements suggested.

Importantly, can you also update the README to describe how writes works (using a temp file). How read/ write work together (or not) etc.

@oeph
Copy link
Contributor Author

oeph commented Feb 15, 2023

@markjschreiber thank you for the comments. I will have a look at them today

@markjschreiber
Copy link
Contributor

Don't forget to include updated information in the README about how write works and what users should expect.

@oeph
Copy link
Contributor Author

oeph commented Feb 16, 2023

Yes, I will do this. I just wanted to do the timeout beforehand and update the README as last step.

Philipp Oehme added 3 commits February 16, 2023 09:42
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@markjschreiber markjschreiber merged commit 11d868c into awslabs:main Feb 23, 2023
@oeph oeph deleted the feature/add_nio_write_operations branch February 27, 2023 10:57
@markjschreiber
Copy link
Contributor

@oeph, many thanks for the contribution!

markjschreiber pushed a commit that referenced this pull request Sep 19, 2023
* fixed issues #3 and #5 and little cleanup

* Configuration description improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants