🐛 Source Salesforce: Use Apparent Encoding #19294

PierreKerschgens · 2022-11-10T17:58:37Z

What

Hey everybody,

I opened an issue about inconsistent encodings in my Salesforce raw output data.

Fixes: #14659
Fixes: #19289

How

Before Airbyte starts fetching data it checks Salesforce's response for the encoding.

The previous (and at least not for me) working order of encoding determination was:

value of Salesforce response header
or interpretation of Salesforce response body (which contains the data we try to get)
or the fallback value DEFAULT_ENCODING = "utf-8"

I inverted the order of encoding determination to:

interpretation of Salesforce response body (which contains the data we try to get)
or value of Salesforce response header
or the fallback value DEFAULT_ENCODING = "utf-8"

My sample output files are consistent and properly encoded now.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
If new credentials are required for use in CI, add them to GSM. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unfortunately I don't have sufficient permissions to perform 1 of the 6 integration test.
The acceptance-test didn't fully work because my Salesforce sandbox account has no data in the 'Asset' table.
Apart from that it looks good.

Unit

========================================================================================================== warnings summary ===========================================================================================================
source_salesforce/exceptions.py:13
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/source_salesforce/exceptions.py:13: DeprecationWarning: Call to deprecated class AirbyteLogger. (Use logging.getLogger('airbyte') instead) -- Deprecated since version 0.1.47.
logger = AirbyteLogger()

source_salesforce/rate_limiting.py:21
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/source_salesforce/rate_limiting.py:21: DeprecationWarning: Call to deprecated class AirbyteLogger. (Use logging.getLogger('airbyte') instead) -- Deprecated since version 0.1.47.
logger = AirbyteLogger()

airbyte-integrations/connectors/source-salesforce/unit_tests/api_test.py: 49 warnings
airbyte-integrations/connectors/source-salesforce/unit_tests/test_memory.py: 4 warnings
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/source_salesforce/source.py:82: DeprecationWarning: Call to deprecated class TokenAuthenticator. (Use airbyte_cdk.sources.streams.http.requests_native_auth.TokenAuthenticator instead) -- Deprecated since version 0.1.20.
authenticator = TokenAuthenticator(sf_object.access_token)

airbyte-integrations/connectors/source-salesforce/unit_tests/api_test.py: 102 warnings
airbyte-integrations/connectors/source-salesforce/unit_tests/test_memory.py: 8 warnings
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/.venv/lib/python3.9/site-packages/deprecated/classic.py:173: DeprecationWarning: Call to deprecated class HttpAuthenticator. (Use requests.auth.AuthBase instead) -- Deprecated since version 0.1.20.
return old_new1(cls, *args, **kwargs)

airbyte-integrations/connectors/source-salesforce/unit_tests/api_test.py: 53 warnings
airbyte-integrations/connectors/source-salesforce/unit_tests/test_memory.py: 4 warnings
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/.venv/lib/python3.9/site-packages/airbyte_cdk/sources/streams/http/http.py:43: DeprecationWarning: Call to deprecated class NoAuth. (Set authenticator=None instead) -- Deprecated since version 0.1.20.
self._authenticator: HttpAuthenticator = NoAuth()

airbyte-integrations/connectors/source-salesforce/unit_tests/api_test.py::test_check_connection_rate_limit[403-login_json_resp0-200-discovery_resp_json0-API Call limit is exceeded]
airbyte-integrations/connectors/source-salesforce/unit_tests/api_test.py::test_check_connection_rate_limit[200-login_json_resp1-403-discovery_resp_json1-An error occurred: [{"errorCode": "FORBIDDEN", "message": "You do not have enough permissions"}]]
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/source_salesforce/api.py:271: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
self.logger.warn(f"http error body: {err.response.text}")

airbyte-integrations/connectors/source-salesforce/unit_tests/api_test.py::test_check_connection_rate_limit[403-login_json_resp0-200-discovery_resp_json0-API Call limit is exceeded]
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/source_salesforce/source.py:51: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
logger.warn(f"API Call limit is exceeded. Error message: '{error_data.get('message')}'")

airbyte-integrations/connectors/source-salesforce/unit_tests/api_test.py::test_rate_limit_bulk
airbyte-integrations/connectors/source-salesforce/unit_tests/api_test.py::test_rate_limit_rest
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/source_salesforce/source.py:142: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
logger.warn(f"API Call limit is exceeded. Error message: '{error_data.get('message')}'")

airbyte-integrations/connectors/source-salesforce/unit_tests/discovery_test.py::test_discover_only_queryable
/Users/trupke/devel/git/airbyte/airbyte-integrations/connectors/source-salesforce/source_salesforce/api.py:239: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
self.logger.warn(f"Stream {stream_object['name']} is not queryable and will be ignored.")

-- Docs: https://docs.pytest.org/en/stable/warnings.html

Results (66.47s):
71 passed

Integration

======================================================================================================= short test summary info =======================================================================================================
FAILED integration_tests/bulk_error_test.py::test_failed_jobs_with_successful_switching[failed_switching] - Failed: insufficient access rights on cross-reference id is missing from captured log

Results (168.41s):
5 passed
1 failed
- airbyte-integrations/connectors/source-salesforce/integration_tests/bulk_error_test.py:58 test_failed_jobs_with_successful_switching[failed_switching]

Acceptance

======================================================================================================= short test summary info =======================================================================================================
FAILED ../../bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: All streams should return some records, streams without records: {'Asset'}

Results (1286.09s):
30 passed
1 failed
- airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py:537 TestBasicRead.test_read[inputs0]

CLAassistant · 2022-11-10T17:58:44Z

All committers have signed the CLA.

vincentkoc · 2022-11-13T11:46:18Z

@PierreKerschgens thanks for your contribution. I'm one of the Airbyte maintainers and will take a look at your PR

…-apparent-encoding

vincentkoc · 2022-11-13T11:48:25Z

/test connector=connectors/source-salesforce

🕑 connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3455126779
✅ connectors/source-salesforce https://github.com/airbytehq/airbyte/actions/runs/3455126779
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           311     69    78%
source_salesforce/rate_limiting.py      22      6    73%
source_salesforce/source.py             94     31    67%
source_salesforce/utils.py               8      7    12%
--------------------------------------------------------
TOTAL                                  596    133    78%
Name                                 Stmts   Miss  Cover
--------------------------------------------------------
source_salesforce/utils.py               8      0   100%
source_salesforce/__init__.py            2      0   100%
source_salesforce/source.py             94      4    96%
source_salesforce/api.py               151     14    91%
source_salesforce/exceptions.py          8      1    88%
source_salesforce/rate_limiting.py      22      3    86%
source_salesforce/streams.py           311     45    86%
--------------------------------------------------------
TOTAL                                  596     67    89%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       139      5    96%   87, 93, 235, 239-240
	 source_acceptance_test/conftest.py                     196     92    53%   35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              394    111    72%   53, 58, 87-95, 100-107, 111-112, 116-117, 293, 331-348, 357-365, 369-374, 380, 413-418, 456-463, 506-508, 511, 576-584, 596-599, 604, 660-661, 667, 670, 706-716, 729-754
	 source_acceptance_test/tests/test_incremental.py       158     14    91%   52-59, 64-77, 240
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 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-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1557    349    78%

Build Passed

Test summary info:

All Passed

vincentkoc · 2022-11-13T12:16:37Z

/publish connector=connectors/source-salesforce run-tests=false

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

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 ▶️

* fix: prefer response body encoding over header encoding * bump: source-salesforce version * Update salesforce.md * auto-bump connector version Co-authored-by: Vincent Koc <[email protected]> Co-authored-by: Octavia Squidington III <[email protected]>

octavia-squidington-iv added bounty community connectors/source/salesforce labels Nov 10, 2022

PierreKerschgens added 2 commits November 10, 2022 19:02

fix: prefer response body encoding over header encoding

1424929

bump: source-salesforce version

d90f957

PierreKerschgens mentioned this pull request Nov 10, 2022

Source Salesforce: expected UTF-8 encoding but got ISO-8859-1 #19289

Closed

vincentkoc changed the title ~~Pierre Kerschgens/fix source salesforce use apparent encoding~~ 🐛 Source Salesforce: Use Apparent Encoding Nov 13, 2022

Merge branch 'master' into PierreKerschgens/fix-source-salesforce-use…

ef2588a

…-apparent-encoding

octavia-squidington-iv added the area/connectors Connector related issues label Nov 13, 2022

vincentkoc self-requested a review November 13, 2022 12:01

vincentkoc self-assigned this Nov 13, 2022

Update salesforce.md

39cded0

octavia-squidington-iv added the area/documentation Improvements or additions to documentation label Nov 13, 2022

vincentkoc approved these changes Nov 13, 2022

View reviewed changes

auto-bump connector version

bbae00f

sajarin added the bounty-M Maintainer program: claimable medium bounty PR label Nov 14, 2022

sajarin merged commit fae29ee into airbytehq:master Nov 14, 2022

PierreKerschgens deleted the PierreKerschgens/fix-source-salesforce-use-apparent-encoding branch November 14, 2022 18:47

github-actions bot mentioned this pull request Nov 15, 2022

Bump helm chart version reference to 0.40.43 #19453

Closed

octavia-squidington-iii mentioned this pull request Nov 18, 2022

Bump Airbyte version from 0.40.18 to 0.40.19 #19579

Merged

PierreKerschgens mentioned this pull request Jul 14, 2023

Source Salesforce: wrong encoding of sync data #27305

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Source Salesforce: Use Apparent Encoding #19294

🐛 Source Salesforce: Use Apparent Encoding #19294

PierreKerschgens commented Nov 10, 2022 •

edited by vincentkoc

Loading

CLAassistant commented Nov 10, 2022 •

edited

Loading

vincentkoc commented Nov 13, 2022

vincentkoc commented Nov 13, 2022 •

edited by github-actions bot

Loading

vincentkoc commented Nov 13, 2022 •

edited by github-actions bot

Loading

🐛 Source Salesforce: Use Apparent Encoding #19294

🐛 Source Salesforce: Use Apparent Encoding #19294

Conversation

PierreKerschgens commented Nov 10, 2022 • edited by vincentkoc Loading

What

How

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

CLAassistant commented Nov 10, 2022 • edited Loading

vincentkoc commented Nov 13, 2022

vincentkoc commented Nov 13, 2022 • edited by github-actions bot Loading

Build Passed

vincentkoc commented Nov 13, 2022 • edited by github-actions bot Loading

PierreKerschgens commented Nov 10, 2022 •

edited by vincentkoc

Loading

CLAassistant commented Nov 10, 2022 •

edited

Loading

vincentkoc commented Nov 13, 2022 •

edited by github-actions bot

Loading

vincentkoc commented Nov 13, 2022 •

edited by github-actions bot

Loading