-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
🎉 New Source: Freshsales #6963
🎉 New Source: Freshsales #6963
Conversation
@tuanchris any possibility to run a sync with a postgres destinaiton and basic normalization on? =) |
@marcosmarxm I actually did that with BigQuery, hope that it is fine too. All seems to be working well but this.
Should I change the boolean schema to integer or what is the preferred way of dealing with this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments, but overall looks good! I'll set up this locally and test. Can you check the comments?
airbyte-integrations/connectors/source-freshsales/source_freshsales/source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-freshsales/source_freshsales/source.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-freshsales/source_freshsales/source.py
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-freshsales/source_freshsales/source.py
Outdated
Show resolved
Hide resolved
@marcosmarxm Running all good locally with basic normalization to BigQuery now |
thanks @tuanchris going to review and test the connector later today. |
Hi @tuanchris the unit tests are not passing, can you check them? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests are not passing.
================================================================================ short test summary info ================================================================================
FAILED unit_tests/test_source.py::test_check_connection - assert (False,\n ConnectionError(MaxRetryError('HTTPSConnectionPool(host="%3cmagicmock%20name=\'mock.__getitem__()\'%20id=\'...
FAILED unit_tests/test_streams.py::test_request_params - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_next_page_token - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_parse_response - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_request_headers - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_http_method - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_should_retry[HTTPStatus.OK-False] - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_should_retry[HTTPStatus.BAD_REQUEST-False] - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_should_retry[HTTPStatus.TOO_MANY_REQUESTS-True] - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_should_retry[HTTPStatus.INTERNAL_SERVER_ERROR-True] - TypeError: __init__() missing 1 required positional argument: 'domain_name'
FAILED unit_tests/test_streams.py::test_backoff_time - TypeError: __init__() missing 1 required positional argument: 'domain_name'
airbyte-integrations/connectors/source-freshsales/unit_tests/test_source.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-freshsales/unit_tests/test_source.py
Outdated
Show resolved
Hide resolved
all fixed @marcosmarxm |
Unit tests are passing but Integration tests aren't.
Make sure |
@marcosmarxm I'm stuck here, can you help take a look? Two problems that I'm facing is with the following message running integration test:
So it seems that the results from Freshsales gave more columns than in the doc. I have consolidated all possible columns, change the type to Another warning that I'm seeing is like follow but I cannot debug it also.
|
@marcosmarxm Problem found. response was returning tokens, which changed with different requests. How do I remove this column? Removing it from the schema doesn't seem to help. |
@tuanchris the problem is not the token data but the connector you built doesnt run a second full refresh properly. The first sync: works and ingest data |
@marcosmarxm I couldn't reproduce the failed attempt you show locally. Running two consecutive sync works for me Furthermore, running the following test script for five minutes doesn't seem to throw any error:
With the following config setting:
The integration test failed and I got this diff (only the token is different). I still think that this is the culprit. |
@marcosmarxm I removed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @tuanchris now integration test are working.
* initial commit * finish implementing full_refresh * add other tables * cleaning up * add docs and use requests_native_auth * fix function return different number of values * change package author * fix schema * fix schema bug * linting * fix unit test * clean up * add null for schemas * remove fc_widget_collaboration col
* initial commit * finish implementing full_refresh * add other tables * cleaning up * add docs and use requests_native_auth * fix function return different number of values * change package author * fix schema * fix schema bug * linting * fix unit test * clean up * add null for schemas * remove fc_widget_collaboration col
What
How
Describe the solution
Connection ran successfully in local env.
Stream settings
Local JSON files
Example output
Recommended reading order
x.java
y.python
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described hereConnector Generator
-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