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

Postgres file reader now assumes UTF8 #15697

Merged
merged 2 commits into from
Aug 16, 2022
Merged

Postgres file reader now assumes UTF8 #15697

merged 2 commits into from
Aug 16, 2022

Conversation

evantahler
Copy link
Contributor

@evantahler evantahler commented Aug 16, 2022 •
edited
Loading

This fixes a failing test, :airbyte-integrations:bases:base-java:spotbugsMain

So,
bailey-computer1

but I think that assuming that all of our users/servers will be using a UTF8 is reasonable in 2022...

@evantahler evantahler changed the title Postgres file reader assumes UTF8 Postgres file reader now assumes UTF8 Aug 16, 2022
@evantahler evantahler marked this pull request as ready for review August 16, 2022 21:28
@evantahler evantahler requested a review from a team as a code owner August 16, 2022 21:28
@edgao
Copy link
Contributor

edgao commented Aug 16, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2871533966
❌ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2871533966
🐛 https://gradle.com/s/nsy2xuq3hl5tc

Build Failed

Test summary info:

Could not find result summary

@evantahler
Copy link
Contributor Author

evantahler commented Aug 16, 2022 •
edited
Loading

Hmm... @edgao I don't really get how my changes caused those compilation errors. Any tips?

Maybe it's transient... trying again.

@evantahler
Copy link
Contributor Author

evantahler commented Aug 16, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2871588185
❌ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2871588185
🐛 https://gradle.com/s/4kni4fitq7qxw

Build Failed

Test summary info:

Could not find result summary

@edgao
Copy link
Contributor

edgao commented Aug 16, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-postgres

🕑 connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2871614776
✅ connectors/source-postgres https://github.com/airbytehq/airbyte/actions/runs/2871614776
No Python unittests run

Build Passed

Test summary info:

All Passed

@github-actions github-actions bot added the area/connectors Connector related issues label Aug 16, 2022
@edgao
Copy link
Contributor

edgao commented Aug 16, 2022

looks like master's PostgresSource is just broken, copied in the fix from another branch https://github.com/airbytehq/airbyte/pull/15317/files#diff-f14bc8e9216a505150004f54d41ca7aeeba9a9460fc4a660b11225dd7abb274f

@evantahler
Copy link
Contributor Author

cool, cool... thanks for the fix!

@ryankfu
Copy link
Contributor

ryankfu commented Aug 16, 2022

Ed covered it already but if you go into the failed Postgres Source integration test and click on Annotations > integration-test you can see the error that there was an issue with import com.google.common.collect.ImmutableMap.Builder; was missing for posterity

It existed before in PostgresSource.java https://github.com/airbytehq/airbyte/blob/master/airbyte-integrations/connectors/source-postgres/src/main/java/io/airbyte/integrations/source/postgres/PostgresSource.java#L18 so not sure how it got removed

@edgao
Copy link
Contributor

edgao commented Aug 16, 2022

tests are green; merging

@edgao
Copy link
Contributor

edgao commented Aug 16, 2022

(not going to publish, since I think this is actually default behavior - if anyone disagrees just LMK)

@edgao edgao merged commit 5d35c7f into master Aug 16, 2022
@edgao edgao deleted the evan/fix-spotless-encoding branch August 16, 2022 23:45
@evantahler evantahler mentioned this pull request Aug 17, 2022
Merged
@evantahler
Copy link
Contributor Author

Looks like mySQl has the same problem #15709

@octavia-squidington-iii octavia-squidington-iii mentioned this pull request Aug 19, 2022
Closed
@evantahler evantahler mentioned this pull request Aug 22, 2022
Merged
rodireich pushed a commit that referenced this pull request Aug 25, 2022
@evantahler @edgao @rodireich
Postgres file reader now assumes UTF8 (#15697)
be1f5cb 
* Postgres file reader assumes UTF8

* fix build :/

Co-authored-by: Edward Gao <edward.gao@airbyte.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rodireich rodireich rodireich approved these changes

@tuliren tuliren tuliren approved these changes

Assignees
No one assigned
Labels
area/connectors Connector related issues connectors/source/postgres
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@evantahler @edgao @ryankfu @rodireich @tuliren @octavia-squidington-iii