Replace unicodecsv with standard csv library #31693
Merged
Conversation
This file contains 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
boring-cyborg
bot
added
area:providers
provider:Apache
provider:microsoft-azure
uranusjr
approved these changes
Jun 5, 2023
tests/providers/microsoft/azure/transfers/test_oracle_to_azure_data_lake.py
Show resolved
Hide resolved
dstandish
force-pushed
the
remove-unicode-csv
branch
2 times, most recently
from
df51506 to
bdf982b
Compare
unicodecsv appears to be missing a license which can cause trouble (see jdunck/python-unicodecsv#80) And it appears that this library may no longer be required
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
dstandish
force-pushed
the
remove-unicode-csv
branch
from
bdf982b to
8a1ce95
Compare
ephraimbuddy
approved these changes
Jun 7, 2023
jedcunningham
approved these changes
Jun 7, 2023
eladkal
added
the
type:misc/internal
eladkal
pushed a commit
that referenced
this pull request
Jun 8, 2023
unicodecsv appears to be missing a license which can cause trouble (see jdunck/python-unicodecsv#80) And it appears that this library may no longer be required. (cherry picked from commit fbeb01c)
potiuk
added a commit
to potiuk/airflow
that referenced
this pull request
Jun 9, 2023
Removing unicodescv as dependency invites problems when users will use older hive, google, microsoft providers, because they were using unicodecsv, but they did not declare it as dependency (it was a transitive dependency of the "apache-airflow" package). It has been removed in apache#31693 Unicodecsv has very low footprint so this is not a problem to keep it. The dependency misses license in it's package, therefore we add the licence in our "licences" folder.
potiuk
added a commit
that referenced
this pull request
Jun 9, 2023
Removing unicodescv as dependency invites problems when users will use older hive, google, microsoft providers, because they were using unicodecsv, but they did not declare it as dependency (it was a transitive dependency of the "apache-airflow" package). It has been removed in #31693 Unicodecsv has very low footprint so this is not a problem to keep it. The dependency misses license in it's package, therefore we add the licence in our "licences" folder.
potiuk
pushed a commit
that referenced
this pull request
Jun 9, 2023
unicodecsv appears to be missing a license which can cause trouble (see jdunck/python-unicodecsv#80) And it appears that this library may no longer be required. (cherry picked from commit fbeb01c)
potiuk
added a commit
that referenced
this pull request
Jun 9, 2023
Removing unicodescv as dependency invites problems when users will use older hive, google, microsoft providers, because they were using unicodecsv, but they did not declare it as dependency (it was a transitive dependency of the "apache-airflow" package). It has been removed in #31693 Unicodecsv has very low footprint so this is not a problem to keep it. The dependency misses license in it's package, therefore we add the licence in our "licences" folder. (cherry picked from commit a853233)
62 tasks
86 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
unicodecsv appears to be missing a license which can cause trouble (see jdunck/python-unicodecsv#80)
And it appears that this library may no longer be required
I have not tested this with any of the actual services... just coding it up based on how it's supposed to work AFAICT
Basically, previously you needed to open the file in mode
wband then you set the encoding on the writer object. Then the writer object would accept strings and encode them before passing to the file handler.And now, you open the file in text mode (optionally you can set an encoding) and then you still pass it a string.
There were some cases (sql to gcs) where the file handler was sometimes used directly (separte from the csv writer object) and there we previously needed to encode to utf-8 first -- that's why i removed those "encode" calls.
There were a couple instances where the handler previously opened in
wmode and i'm not sure how that actually worked because it doesn't seem to work for me. Possible those cases were actually broken? (mssql-to-hive, vertica-to-hive, vertica-to-mysql) Also possible i am just missing something on that issue.