🐛 Fix Intercom stream base url in check connection

[mrhallak]

What

Hello Airbyte team! I was giving the tool a shot and wanted to use Intercom as a source. I noticed the extra slash at the end of the URL through the logs I got because I was not able to connect.

How

The solution is pretty simple - just removing the slash at the end of the base url.

Recommended reading order

1. python

🚨 User Impact 🚨

There shouldn't be any breaking changes here

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

• Community member? 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
  • docs/SUMMARY.md
  • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • docs/integrations/README.md
  • airbyte-integrations/builds.md
• 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 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 by running the /publish command described here
• After the new connector version is published, connector version bumped in the seed directory 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

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

Note some tests failed - it seems the Intercom token I have does not grant me access to everything which is expected because I'm using a token provided by my organization

Acceptance

Results (17.32s):
17 passed
5 failed
- ../airbyte/source_acceptance_test/source_acceptance_test/tests/test_core.py:144 TestConnection.test_check[inputs0]
- ../airbyte/source_acceptance_test/source_acceptance_test/tests/test_core.py:355 TestBasicRead.test_read[inputs0]
- ../airbyte/source_acceptance_test/source_acceptance_test/tests/test_full_refresh.py:39 TestFullRefresh.test_sequential_reads[inputs0]
- ../airbyte/source_acceptance_test/source_acceptance_test/tests/test_incremental.py:81 TestIncremental.test_two_sequential_reads[inputs0]
- ../airbyte/source_acceptance_test/source_acceptance_test/tests/test_incremental.py:105 TestIncremental.test_state_with_abnormally_large_values[inputs0]

[CLAassistant]

All committers have signed the CLA.

[sherifnada]

this is concatenated with stream paths below to form https://api.intercom.io/<path> -- is it safe to do this?

[marcosmarxm]

For reading stream data doesn't matter urljoin will parse correctly with / or without.

The problem happens in the check connection function where is concatenating using f-string and not urljoin.

airbyte/airbyte-integrations/connectors/source-intercom/source_intercom/source.py

Lines 416 to 417 in 266a491

	url = f"{IntercomStream.url_base}/tags"
	auth_headers = {"Accept": "application/json", **authenticator.get_auth_header()}

[mrhallak]

@sherifnada @marcosmarxm Would you guys prefer it if I change it to a urljoin?

[sherifnada]

That would be better imo

[mrhallak]

Updated!

[marcosmarxm]

@mrhallak can you sign the CLA?

[mrhallak]

@mrhallak can you sign the CLA?

Done!

[marcosmarxm]

/test connector=connectors/source-intercom repo=mrhallak/airbyte