🐛 Source S3: fix datetime format string in FileStream - #23195
Merged
Evan Tahler (evantahler) merged 12 commits intoMar 16, 2023
Merged
🐛 Source S3: fix datetime format string in FileStream#23195Evan Tahler (evantahler) merged 12 commits into
Evan Tahler (evantahler) merged 12 commits into
Conversation
Octavia Squidington III (octavia-squidington-iii)
added
area/connectors
Contributor
Author
|
Attaching the output of |
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
|
/test connector=connectors/source-s3
Build FailedTest summary info: |
Contributor
Author
|
Nataly Merezhuk (@natalyjazzviolin) could you give this another run? I updated the integration tests. |
Contributor
Author
|
Hi Nataly Merezhuk (@natalyjazzviolin), pinging you again as we're still seeing issues because of this. Do let me know if there's anything more I can do to get this merged. |
Contributor
|
/test connector=connectors/source-s3
Build FailedTest summary info: |
Contributor
Author
|
The latest commit should fix the issue of comparing timezone-aware datetime objects with unaware objects. Attaching unit test logs for this commit. |
Contributor
|
/test connector=connectors/source-s3
Build PassedTest summary info: |
Contributor
|
sunny (@sh4sh) 👍 from me, assuming the tests pass. |
Edward Gao (edgao)
approved these changes
Mar 13, 2023
Edward Gao (edgao)
left a comment
Edward Gao (edgao)
left a comment
Contributor
There was a problem hiding this comment.
is this change switching the TZ from +0000 to Z? That seems right to me. Some destinations (e.g. snowflake) assume that the tz offset is either Z or +/- HH:MM (i.e. with a colon), e.g. https://github.com/airbytehq/oncall/issues/1476.
Evan Tahler (evantahler)
suggested changes
Mar 13, 2023
Comment on lines
+1
to
+2
| {"stream": "test", "data": {"id": 0, "fullname_and_valid": {"fullname": "cfjwIzCRTL", "valid": false}, "_ab_additional_properties": {}, "_ab_source_file_last_modified": "2022-05-11T11:54:11Z", "_ab_source_file_url": "test_sample.avro"}, "emitted_at": 10000000} | ||
| {"stream": "test", "data": {"id": 1, "fullname_and_valid": {"fullname": "LYOnPyuTWw", "valid": true}, "_ab_additional_properties": {}, "_ab_source_file_last_modified": "2022-05-11T11:54:11Z", "_ab_source_file_url": "test_sample.avro"}, "emitted_at": 10000000} |
Contributor
There was a problem hiding this comment.
Yep! this looks like the change we want!
|
|
||
| | Version | Date | Pull Request | Subject | | ||
| |:--------|:-----------|:----------------------------------------------------------------------------------------------------------------|:-------------------------------------------------------------------------------------------| | ||
| | 1.0.3 | 2023-03-06 | [23195](https://github.com/airbytehq/airbyte/pull/23195) | Fix datetime format string | |
Contributor
There was a problem hiding this comment.
Oliver Meyer (@olivermeyer) this PR is one file change away from being ready to merge - You also need to bump the version in the Dockerfile.
Contributor
Author
There was a problem hiding this comment.
Done :)
Contributor
|
Oh no Oliver Meyer (@olivermeyer) - we've got merge conflicts again. |
Contributor
|
/test connector=connectors/source-s3
Build PassedTest summary info: |
Evan Tahler (evantahler)
approved these changes
Mar 16, 2023
Evan Tahler (evantahler)
left a comment
Evan Tahler (evantahler)
left a comment
Contributor
There was a problem hiding this comment.
Tests passing, let's go!
Contributor
|
/publish connector=connectors/source-s3
if you have connectors that successfully published but failed definition generation, follow step 4 here |
Adrienne Vermorel (adriennevermorel)
pushed a commit
to adriennevermorel/airbyte
that referenced
this pull request
Mar 17, 2023
* Fix datetime format string in FileStream * Update changelog * Fix integration tests * Localize datetime objects * Bump Dockerfile version * auto-bump connector version --------- Co-authored-by: Nataly Merezhuk <65251165+natalyjazzviolin@users.noreply.github.com> Co-authored-by: sh4sh <6833405+sh4sh@users.noreply.github.com> Co-authored-by: Evan Tahler <evan@airbyte.io> Co-authored-by: Augustin <augustin@airbyte.io> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Ella Rohm-Ensing (erohmensing)
pushed a commit
that referenced
this pull request
Mar 22, 2023
* Fix datetime format string in FileStream * Update changelog * Fix integration tests * Localize datetime objects * Bump Dockerfile version * auto-bump connector version --------- Co-authored-by: Nataly Merezhuk <65251165+natalyjazzviolin@users.noreply.github.com> Co-authored-by: sh4sh <6833405+sh4sh@users.noreply.github.com> Co-authored-by: Evan Tahler <evan@airbyte.io> Co-authored-by: Augustin <augustin@airbyte.io> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Ella Rohm-Ensing (erohmensing)
pushed a commit
that referenced
this pull request
Mar 22, 2023
* Fix datetime format string in FileStream * Update changelog * Fix integration tests * Localize datetime objects * Bump Dockerfile version * auto-bump connector version --------- Co-authored-by: Nataly Merezhuk <65251165+natalyjazzviolin@users.noreply.github.com> Co-authored-by: sh4sh <6833405+sh4sh@users.noreply.github.com> Co-authored-by: Evan Tahler <evan@airbyte.io> Co-authored-by: Augustin <augustin@airbyte.io> Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes #22919
The
FileStreamhas adatetime_format_stringattribute which is used to format the cursor field (here). At some point during the sync, Airbyte seems to validate the format of this field, resulting in errors such as this:The
FileStreamreturns a datetime string formatted as1970-01-01T00:00+0000, whereas the validation expects a string formatted as1970-01-01T00:00Z.How
Fix the datetime format string.
Pre-merge Checklist
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing/publishcommand described here