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

Source salesforce: handle japanese characters #17001

Merged
merged 5 commits into from
Sep 22, 2022
Merged

Source salesforce: handle japanese characters #17001

merged 5 commits into from
Sep 22, 2022

Conversation

davydov-d
Copy link
Collaborator

@davydov-d davydov-d commented Sep 21, 2022 •
edited
Loading

What

https://github.com/airbytehq/oncall/issues/454
Do not decode response when writing to file since the chunk can start/end right in the middle of a 2/3/4 byte sequence which represents a single character thus makes text unreadable

@github-actions github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Sep 21, 2022
@davydov-d
Copy link
Collaborator Author

davydov-d commented Sep 21, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-salesforce

🕑 connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3100650903
❌ connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3100650903
🐛 https://gradle.com/s/6hwypioebxj6k

Build Failed

Test summary info:

Could not find result summary

@davydov-d
Copy link
Collaborator Author

hey @Nakachi-S
if you dont mind I have created this PR based on yours but also added changes that would make all the tests pass

@davydov-d
Copy link
Collaborator Author

davydov-d commented Sep 21, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-salesforce

🕑 connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3100764785
❌ connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3100764785
🐛 https://gradle.com/s/cvp6tw6dolh5c

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_incremental.py::TestIncremental::test_two_sequential_reads[inputs0]
FAILED test_incremental.py::TestIncremental::test_read_sequential_slices[inputs0]
=================== 2 failed, 28 passed in 203.61s (0:03:23) ===================

@vincentkoc
Copy link
Contributor

@davydov-d cant see where this code is different from the original PR?

@davydov-d
Copy link
Collaborator Author

@davydov-d cant see where this code is different from the original PR?

@koconder it differs in the source.py file but tests keep on failing so I will go on working on the PR

@davydov-d
#454 source salesforce: adjust public interface to CDK, do not take i…
5d75abd 
…nto account state when choosing the API type
@davydov-d
Copy link
Collaborator Author

davydov-d commented Sep 22, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-salesforce

🕑 connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3104092371
❌ connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3104092371
🐛 https://gradle.com/s/3dn266qbaqcxc

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_incremental.py::TestIncremental::test_read_sequential_slices[inputs0]
=================== 1 failed, 29 passed in 206.48s (0:03:26) ===================

@davydov-d
Copy link
Collaborator Author

davydov-d commented Sep 22, 2022 •
edited by github-actions bot
Loading

/test connector=connectors/source-salesforce

🕑 connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3104221964
✅ connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3104221964
Python tests coverage:

Name                                 Stmts   Miss  Cover
--------------------------------------------------------
source_salesforce/__init__.py            2      0   100%
source_salesforce/exceptions.py          8      1    88%
source_salesforce/api.py               151     19    87%
source_salesforce/streams.py           307     71    77%
source_salesforce/rate_limiting.py      22      6    73%
source_salesforce/source.py             88     34    61%
source_salesforce/utils.py               8      7    12%
--------------------------------------------------------
TOTAL                                  586    138    76%
Name                                 Stmts   Miss  Cover
--------------------------------------------------------
source_salesforce/utils.py               8      0   100%
source_salesforce/__init__.py            2      0   100%
source_salesforce/source.py             88      7    92%
source_salesforce/api.py               151     14    91%
source_salesforce/exceptions.py          8      1    88%
source_salesforce/streams.py           307     41    87%
source_salesforce/rate_limiting.py      22      3    86%
--------------------------------------------------------
TOTAL                                  586     66    89%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          10      4    60%   15-18
	 source_acceptance_test/config.py                        83      6    93%   78-80, 84-86
	 source_acceptance_test/conftest.py                     164    164     0%   6-282
	 source_acceptance_test/plugin.py                        48     48     0%   6-104
	 source_acceptance_test/tests/test_core.py              329    111    66%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 374-376, 379, 439-448, 477-478, 484, 487, 520-530, 543-568, 573-577
	 source_acceptance_test/tests/test_full_refresh.py       52      2    96%   34, 65
	 source_acceptance_test/tests/test_incremental.py       152     26    83%   21-23, 29-31, 36-43, 48-61, 239, 250-258
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     17    78%   15-16, 24-30, 47-54, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-67, 70-72, 75-77, 80-82, 85-87, 90-92, 95-113, 147-149
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1358    466    66%

Build Passed

Test summary info:

All Passed

vincentkoc
vincentkoc approved these changes Sep 22, 2022
View reviewed changes
Copy link
Contributor

@vincentkoc vincentkoc left a comment

Choose a reason for hiding this comment

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

LGTM

@davydov-d
Copy link
Collaborator Author

davydov-d commented Sep 22, 2022 •
edited by github-actions bot
Loading

/publish connector=connectors/source-salesforce

🕑 Publishing the following connectors:
connectors/source-salesforce
https://github.com/airbytehq/airbyte/actions/runs/3104841243


Connector Did it publish? Were definitions generated?
connectors/source-salesforce ✅ ✅

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@davydov-d davydov-d merged commit 596a436 into master Sep 22, 2022
@davydov-d davydov-d deleted the ddavydov/#454-oncall-source-salesforce-japanese-characters-encoding branch September 22, 2022 11:27
@vincentkoc vincentkoc mentioned this pull request Sep 22, 2022
Closed
14 tasks
This was referenced Sep 22, 2022
Closed
Merged
@alafanechere alafanechere mentioned this pull request Sep 25, 2022
Closed
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
@davydov-d @octavia-squidington-iii @robbinhan
Source salesforce: handle japanese characters (airbytehq#17001)
d0eb791 
* airbytehq#454 oncall source salesforce: handle japanese characters

* source salesforce: upd changelog

* source salesforce: flake fix

* airbytehq#454 source salesforce: adjust public interface to CDK, do not take into account state when choosing the API type

* auto-bump connector version [ci skip]

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
@davydov-d @octavia-squidington-iii @jhammarstedt
Source salesforce: handle japanese characters (airbytehq#17001)
f04c90c 
* airbytehq#454 oncall source salesforce: handle japanese characters

* source salesforce: upd changelog

* source salesforce: flake fix

* airbytehq#454 source salesforce: adjust public interface to CDK, do not take into account state when choosing the API type

* auto-bump connector version [ci skip]

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vincentkoc vincentkoc vincentkoc approved these changes

@bazarnov bazarnov bazarnov approved these changes

@pedroslopez pedroslopez Awaiting requested review from pedroslopez

@roman-yermilov-gl roman-yermilov-gl Awaiting requested review from roman-yermilov-gl

Assignees
No one assigned
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/salesforce
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@davydov-d @vincentkoc @bazarnov @octavia-squidington-iii