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.

Sign up for GitHub

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

Jump to bottom

distributed s3 write text #1631

Merged
merged 27 commits into from
Oct 6, 2022
Merged

distributed s3 write text #1631

merged 27 commits into from
Oct 6, 2022

Conversation

LeonLuttenberger
Copy link
Contributor

Feature or Bugfix

  • Feature

Detail

  • Adding distributed versions of s3.write_csv and s3.write_json

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@aws aws deleted a comment from malachi-constant Sep 23, 2022
@aws aws deleted a comment from malachi-constant Sep 23, 2022
@aws aws deleted a comment from malachi-constant Sep 23, 2022
@aws aws deleted a comment from malachi-constant Sep 23, 2022
@aws aws deleted a comment from malachi-constant Sep 23, 2022
@aws aws deleted a comment from malachi-constant Sep 23, 2022
@aws aws deleted a comment from malachi-constant Sep 23, 2022
@aws aws deleted a comment from malachi-constant Sep 23, 2022
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 28926aa
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 28926aa
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 28926aa
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant

This comment was marked as outdated.

Sign in to view
@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 4075bfb
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 4075bfb
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 4075bfb
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 9ccb85c
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 9ccb85c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: d5ac25e
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: d5ac25e
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: d5ac25e
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido
jaidisido approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@jaidisido jaidisido 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, there is only comment that needs addressing please

awswrangler/s3/_write_dataset.py
@@ -299,7 +298,7 @@ def write_partitions(df: pd.DataFrame) -> Tuple[List[str], Dict[str, List[str]]]
table_type=table_type,
transaction_id=transaction_id,
bucketing_info=None,
filename_prefix=uuid.uuid4().hex,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unrelated to your PR but since you are removing uuid4() as an input in favour of filename_prefix (which I agree with), I am just wondering if this won't have a knock on effect on _to_parquet. Specifically, the hex default is set for to_parquet here but not here

Copy link
Contributor

Choose a reason for hiding this comment

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

Hopefully the tests catch the regression but yes we should get rid of mutable defaults. Worst case replace it with a lambda : uuid.uuid4().hex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that prefix should be consistent for to_parquet and to_parquet_distributed.

However, this might not be causing an impact because the filename prefix is also set here? I'll need to check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just ran test_to_parquet_filename_prefix and it's passing. We should still find a solution that's consistent for code readability at least, but it doesn't seem to impacting the behaviour.

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 9e2d940
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 9e2d940
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 9e2d940
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@LeonLuttenberger LeonLuttenberger merged commit bf58bca into release-3.0.0 Oct 6, 2022
@LeonLuttenberger LeonLuttenberger deleted the feat-3.0/distributed-s3-write-text branch October 6, 2022 14:11
@malachi-constant malachi-constant moved this from In Review to Done in AWS SDK for pandas roadmap Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kukushking kukushking kukushking approved these changes

@jaidisido jaidisido jaidisido approved these changes

@malachi-constant malachi-constant Awaiting requested review from malachi-constant

@cnfait cnfait Awaiting requested review from cnfait

Assignees
No one assigned
Labels
feature
Projects
AWS SDK for pandas roadmap
Done
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@LeonLuttenberger @malachi-constant @kukushking @jaidisido