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

Java CDK: Staging destinations include timezone in extracted_at in CSV file #35313

Merged
merged 2 commits into from
Mar 1, 2024
Merged

Java CDK: Staging destinations include timezone in extracted_at in CSV file #35313

merged 2 commits into from
Mar 1, 2024

Conversation

edgao
Copy link
Contributor

@edgao edgao commented Feb 14, 2024 •
edited
Loading

Modify how we write staging files, so that the extracted_at timestamp includes an offset. This fixes destination-snowflake writing extracted_at in local timezone instead of UTC. (see #35308 for the migration code.)

See stacked PRs for tests. I'm just relying on our T+D tests to catch any regressions, since they have assertions for extracted_at being correct.

@vercel Vercel
Copy link

vercel bot commented Feb 14, 2024 •
edited
Loading

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 Mar 1, 2024 7:11pm

@edgao Graphite App
Copy link
Contributor Author

edgao commented Feb 14, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @edgao and the rest of your teammates on Graphite Graphite

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues CDK Connector Development Kit connectors/destination/snowflake labels Feb 14, 2024
@github-actions GitHub Actions
Copy link
Contributor

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
  • If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@edgao edgao force-pushed the edgao/staging_csv_timestamp_timezone branch from 027b08c to 45444b4 Compare February 14, 2024 23:40
@octavia-squidington-iii octavia-squidington-iii removed the area/connectors Connector related issues label Feb 14, 2024
@edgao edgao mentioned this pull request Feb 14, 2024
Merged
@edgao edgao force-pushed the edgao/destination_state_table branch from b7b4c3d to 67f0f17 Compare February 15, 2024 00:14
@edgao edgao force-pushed the edgao/staging_csv_timestamp_timezone branch from 350f326 to 9b0bca9 Compare February 15, 2024 00:14
@edgao edgao mentioned this pull request Feb 15, 2024
Merged
edgao
edgao commented Feb 15, 2024
View reviewed changes
...stinations/src/test/java/io/airbyte/cdk/integrations/destination/s3/csv/S3CsvWriterTest.java Outdated
2b95a13f-d54f-4370-a712-1c7bf2716190,"{""bar"":84}",%s\r
"""
f6767f7d-ce1e-45cc-92db-2ad3dfdd088e,"{""foo"":73}",1970-01-01T00:00:01.234Z\r
2b95a13f-d54f-4370-a712-1c7bf2716190,"{""bar"":84}",1970-01-01T00:00:02.345Z\r
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note the Z offset suffix. Previously this was not present.

This was referenced Feb 15, 2024
Merged
Merged
@edgao edgao force-pushed the edgao/destination_state_table branch 5 times, most recently from 34a4fb3 to fa52885 Compare February 20, 2024 22:23
@edgao edgao force-pushed the edgao/staging_csv_timestamp_timezone branch 3 times, most recently from 9f7d650 to 863ef4a Compare February 21, 2024 20:34
@edgao edgao force-pushed the edgao/destination_state_table branch from be41e16 to b579660 Compare February 21, 2024 20:35
@edgao edgao force-pushed the edgao/staging_csv_timestamp_timezone branch 2 times, most recently from 74528dc to eaad0b5 Compare February 21, 2024 21:32
@edgao edgao force-pushed the edgao/destination_state_table branch 2 times, most recently from 3c448f8 to 9e3d10f Compare February 21, 2024 21:43
@edgao edgao force-pushed the edgao/staging_csv_timestamp_timezone branch 2 times, most recently from 9e0fabb to d128fa1 Compare February 21, 2024 21:52
@edgao edgao mentioned this pull request Feb 23, 2024
Closed
@edgao edgao force-pushed the edgao/staging_csv_timestamp_timezone branch from 51c992b to e944acd Compare February 23, 2024 21:41
@edgao edgao mentioned this pull request Feb 23, 2024
Merged
@edgao edgao force-pushed the edgao/staging_csv_timestamp_timezone branch 4 times, most recently from 291e039 to e074811 Compare February 26, 2024 16:46
This was referenced Feb 26, 2024
Closed
Closed
Closed
@octavia-squidington-iv octavia-squidington-iv requested review from a team February 27, 2024 19:21
@gisripa gisripa force-pushed the edgao/destination_state_table branch 5 times, most recently from 5022d37 to d8ce2d2 Compare February 29, 2024 19:35
@gisripa gisripa force-pushed the edgao/staging_csv_timestamp_timezone branch from e074811 to 51d3784 Compare February 29, 2024 19:38
@gisripa gisripa force-pushed the edgao/destination_state_table branch from d8ce2d2 to d158042 Compare March 1, 2024 18:35
@gisripa gisripa force-pushed the edgao/staging_csv_timestamp_timezone branch from 51d3784 to 6e5f18b Compare March 1, 2024 18:37
Base automatically changed from edgao/destination_state_table to master March 1, 2024 19:04
@gisripa gisripa force-pushed the edgao/staging_csv_timestamp_timezone branch from 6e5f18b to cb16961 Compare March 1, 2024 19:07
@gisripa
Copy link
Contributor

gisripa commented Mar 1, 2024 •
edited by github-actions bot
Loading

/publish-java-cdk

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

@gisripa gisripa merged commit 1685b27 into master Mar 1, 2024
26 checks passed
@gisripa gisripa deleted the edgao/staging_csv_timestamp_timezone branch March 1, 2024 19:28
xiaohansong pushed a commit that referenced this pull request Mar 7, 2024
@edgao @gisripa @xiaohansong
Java CDK: Staging destinations include timezone in extracted_at in CS…
42475bd 
…V file (#35313)

Co-authored-by: Gireesh Sreepathi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbfbell jbfbell jbfbell approved these changes

@gisripa gisripa gisripa approved these changes

Assignees
No one assigned
Labels
CDK Connector Development Kit connectors/destination/snowflake
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@edgao @gisripa @jbfbell @octavia-squidington-iii