Skip to content

S3 Destinations: Move partId generation to AtomicInteger (remove UUID file part names) - #33788

Merged
Evan Tahler (evantahler) merged 13 commits into
masterfrom
evan/s3-ids-atomic
Jan 3, 2024
Merged

S3 Destinations: Move partId generation to AtomicInteger (remove UUID file part names)#33788
Evan Tahler (evantahler) merged 13 commits into
masterfrom
evan/s3-ids-atomic

Conversation

@evantahler

@evantahler Evan Tahler (evantahler) commented Dec 26, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #32229

This PR returns destination-s3 back to using integers for upload file name parts, rather than uuids which was removed to fix a multi-threading issue when the Async framework was introduced to the Redshift destination (#29640). The Redshift destination relies on this same code. The S3 destination was pinned to an older version until this could be fixed.

@vercel

vercel Bot commented Dec 26, 2023 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Jan 3, 2024 6:29pm

@octavia-squidington-iii Octavia Squidington III (octavia-squidington-iii) added the CDK Connector Development Kit label Dec 26, 2023
@github-actions

github-actions Bot commented Dec 26, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Warning

🚨 Connector code freeze is in effect until 2024-01-02. This PR is changing connector code. Please contact the current OC engineers if you want to merge this change to master.

@evantahler
Evan Tahler (evantahler) marked this pull request as ready for review December 26, 2023 21:01
@evantahler
Evan Tahler (evantahler) requested a review from a team as a code owner December 26, 2023 21:01
edgao
Edward Gao (edgao) reviewed Dec 27, 2023
View reviewed changes
Comment thread ...stinations/src/main/java/io/airbyte/cdk/integrations/destination/s3/S3StorageOperations.java Outdated
Comment thread ...stinations/src/main/java/io/airbyte/cdk/integrations/destination/s3/S3StorageOperations.java
Comment thread ...stinations/src/main/java/io/airbyte/cdk/integrations/destination/s3/S3StorageOperations.java Outdated
Comment thread ...stinations/src/main/java/io/airbyte/cdk/integrations/destination/s3/S3StorageOperations.java Outdated
evantahler
Evan Tahler (evantahler) commented Dec 28, 2023
View reviewed changes
Comment thread airbyte-integrations/connectors/destination-s3/metadata.yaml
registries:
cloud:
enabled: true
dockerImageTag: 0.5.1

@evantahler Evan Tahler (evantahler) Dec 28, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note: This PR un-pins destination-s3

@evantahler Evan Tahler (evantahler) changed the title S3 Destinations: Move partId generation to AtomicInteger S3 Destinations: Move partId generation to AtomicInteger (remove UUID file part names) Dec 28, 2023
@evantahler

Evan Tahler (evantahler) commented Dec 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Edit: I forgot to bump the CDK.

^ The dev images above, and locally using airbyte-ci connectors --name destination-s3 build don't seem to produce an image actually using this code!

Starting from scratch (docker system prune -a), airbyte-ci seems to keep producing docker images with the old code. Adding logging near obvious invocations is not logged:

Screenshot 2023-12-28 at 10 35 02 AM

Digging into the Airbyte-CI build commands, this seems sus:

copy /build/distributions/destination-s3-0.50.39.tar /airbyte/destination-s3.tar

What's the 0.50.39 version number? Is there a platform-level binary I need to change? There are some custom build steps in the S3 destination... maybe related?

@evantahler

Evan Tahler (evantahler) commented Dec 28, 2023

Copy link
Copy Markdown
Contributor Author

Works as expected!

Screenshot 2023-12-28 at 10 52 40 AM

@evantahler

Evan Tahler (evantahler) commented Jan 3, 2024

Copy link
Copy Markdown
Contributor Author

Publishing dev images

  • airbyte/destination-s3:0.5.7-dev.12e2eff4ef
  • airbyte/destination-redshift:0.5.7-dev.12e2eff4ef

@evantahler

Evan Tahler (evantahler) commented Jan 3, 2024 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor Author

/publish-java-cdk

🕑 https://github.com/airbytehq/airbyte/actions/runs/7400728040
✅ Successfully published Java CDK version=0.10.2!

@evantahler
Evan Tahler (evantahler) merged commit 785c2f0 into master Jan 3, 2024
@evantahler
Evan Tahler (evantahler) deleted the evan/s3-ids-atomic branch January 3, 2024 19:23
@evantahler Evan Tahler (evantahler) mentioned this pull request Jan 4, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@edgao Edward Gao (edgao) edgao approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/connectors Connector related issues area/documentation Improvements or additions to documentation CDK Connector Development Kit connectors/destination/redshift connectors/destination/s3

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Destination S3: switch back to using incrementing part numbers

3 participants

@evantahler @edgao @octavia-squidington-iii