🐛 Source Marketo: fix encoding error for Lead sync

[CyprienBarbault]

What

This is a fix for an issue I reported -> #20641

When syncing Marketo lead, it appears that those with name containing non-latin character would make the sync fail. In particular we have a Lead whose name is Vietnamese (cf https://en.wikipedia.org/wiki/Vietnamese_name for exemple names)

In fact, the response from the marketo API was being fall-backed to the default encoding ISO-8859-1 instead of utf-8 (cf psf/requests#5445 (comment))

How

Forcing the encoding to utf-8 fixed the issue

Recommended reading order

1. source.py

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

I'm not sure but I don't think so

Pre-merge Checklist

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

Tests

Unit

unit_test_result.txt

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

[CLAassistant]

All committers have signed the CLA.

[vincentkoc]

@CyprienBarbault hope you are well, thanks for your contribution. I’m one of the maintainers here at Airbyte. I'll take a look at your changes, please bear with me.

[vincentkoc]

/test connector=connectors/source-marketo

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

Name                         Stmts   Miss  Cover
------------------------------------------------
source_marketo/__init__.py       2      0   100%
source_marketo/utils.py         33      1    97%
source_marketo/source.py       272     29    89%
------------------------------------------------
TOTAL                          307     30    90%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   56-63, 68-81, 244
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 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       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1609    339    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
=========== 30 passed, 1 skipped, 31 warnings in 2328.01s (0:38:48) ============

[CyprienBarbault]

@koconder Thank you for your help. What are the next steps ?

[vincentkoc]

@CyprienBarbault I’ll need to add some additional changes to your PR to add the version number increase and then run a few more checks, once this is all OK we will release alongside other changes. If Marketo connector is a GA connector I'll also get another code-reviewer to double check the work.

As its just after the holidays, just bear with us as we clear through the backlog 👍

[CyprienBarbault]

Hello @koconder what's the update on this ?

[vincentkoc]

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

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

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

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

[vincentkoc]

Needs additional review from another airbyte reviewer as this is a connector in GA.

[evantahler]

@marcosmarxm this PR published a version of the connector, but it was never published, and it's causing trouble for #22015

[erohmensing]

/test connector=connectors/source-marketo

🕑 connectors/source-marketo https://github.com/airbytehq/airbyte/actions/runs/4048534124
❌ connectors/source-marketo https://github.com/airbytehq/airbyte/actions/runs/4048534124
🐛 https://gradle.com/s/xfxedllmptrgu

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream progr...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:98: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:507: The previous and actual discovered catalogs are identical.
====== 1 failed, 34 passed, 2 skipped, 37 warnings in 2086.60s (0:34:46) =======

> Task :airbyte-integrations:connectors:source-marketo:sourceAcceptanceTest FAILED

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.6/userguide/command_line_interface.html#sec:command_line_warnings

Execution optimizations have been disabled for 1 invalid unit(s) of work during this build to ensure correctness.
Please consult deprecation warnings for more details.
48 actionable tasks: 23 executed, 25 up-to-date

Publishing build scan...
https://gradle.com/s/xfxedllmptrgu

The remote build cache was disabled during the build due to errors.
S3 cache writes: 1, elapsed: 687ms, sent to cache: 448 B

[marcosmarxm]

It was published but not merged, probably this is causing the issue?
@erohmensing let me know if you need assistance here!

[erohmensing]

Yep, that's what's causing the issue. If tests are passing I'm happy to get it in before merging mine, but I also noticed that our #connector-health tests for this source started failing on the same date that it was published, and I'm not sure if it's related. Running the tests now to see what we get

[erohmensing]

To me this seems like a pretty innocuous change, and most likely not causing the issues seen in the tests, as they also appear on the master branch. I'm going to go ahead and get this merged so that the repo reflects whats available on docker hub.

It's more complicated now that someone has already released a newer version, which is what I was trying to avoid - technically these changes aren't in 1.0.0. I'll try to make that reflected as much as possible

[erohmensing]

regular CI won't run on the fork due to lack of access to the PAT, but confirmed that the failure is the same on master, and formatting looks fine. Merging