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.

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: Senseforce [low-code CDK] #18775

Merged
merged 9 commits into from
Nov 9, 2022

Conversation

andnig
Copy link
Contributor

@andnig andnig commented Nov 1, 2022

What

This PR adds the new low-code CDK based Senseforce connector. Senseforce is a european IoT company.

How

This PR uses the lowcode CDK to create a Omnisend connector with the following streams:

  • dataset (full-refresh/incremental): Datasets are the basic data models of any Senseforce installation. This connector now allows to export an arbitrary portion of the Senseforce data models.

Recommended reading order

  1. spec.yaml
  2. omnisend.yaml

🚨 User Impact 🚨

No changes to existing code, except docs/integrations/Readme.md

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/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

Tests

Acceptance

image

@CLAassistant
Copy link

CLAassistant commented Nov 1, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

Hello @andnig, Marcos from Airbyte here 👋 . We received more than 25 new contributions along the weekend. One is yours 🎉 thank so much for! Our team is limited and maybe the review process can take longer than expected. As described in the Airbyte's Hacktoberfest your contribution was submitted before November 2nd and it is eligible to win the prize. The review process will validate other requirements. I ask to you patience until someone from the team review it.

Because I reviewed some contributions for Hacktoberfest so far I saw some common patterns you can check in advance:

  • Make sure you have added connector documentation to /docs/integrations/
  • Remove the file catalog from /integration_tests
  • Edit the sample_config.json inside /integration_tests
  • For the configured_catalog you can use only json_schema: {}
  • Add title to all properties in the spec.yaml
  • Make sure the documentationUrl in the spec.yaml redirect to Airbyte's future connector page, eg: connector Airtable the documentationUrl: https://docs.airbyte.com/integrations/sources/airtable
  • Review now new line at EOF (end-of-file) for all files.

If possible send to me a DM in Slack with the tests credentials, this process will make easier to us run integration tests and publish your connector. If you only has production keys, make sure to create a bootstrap.md explaining how to get the keys.

@marcosmarxm
Copy link
Member

@andnig please request my review again when all points in my previous comment are solved.

@andnig andnig requested a review from marcosmarxm November 3, 2022 13:15
@andnig
Copy link
Contributor Author

andnig commented Nov 3, 2022

@andnig please request my review again when all points in my previous comment are solved.

@marcosmarxm I think all points are resolved, are they? Sorry if not - would you mind pointing me to the issue?

@marcosmarxm
Copy link
Member

Hello! I'm going to be out of the office this Friday and won't be able to review your contribution again today, I return next Monday. So far, most contributions look solid and are almost done to be approved. As said in Chris' comment all contributions made before 2-November are eligible to receive the prize and have 2 weeks to merge the contributions. But I ensure next week we're going to have your contribution merged. If you have questions about the implementation you can send them in #hacktoberfest-2022 in Airbyte's Slack.

Sorry the inconvenience and see you again next week, thank you so much for your contribution!

Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

Some comments, do you have test account to share to run tests?

Comment on lines 1 to 6
# TODO: Define your stream schemas
Your connector must describe the schema of each stream it can output using [JSONSchema](https://json-schema.org).

You can describe the schema of your streams using one `.json` file per stream.

## Static schemas
Copy link
Member

Choose a reason for hiding this comment

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

Remove this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

request_body_data: |
[{"clause": {"type": "timestamp", "operator": 10, "parameters":
[{"value": {{ stream_slice['start_time'] | int * 1000 }} },
{"value": {{ stream_slice['end_time'] | int * 1000 + (86400000 - 1) }} }
Copy link
Member

Choose a reason for hiding this comment

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

What happen if you don't use end_date?

Copy link
Contributor Author

@andnig andnig Nov 8, 2022

Choose a reason for hiding this comment

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

It fetches all the data in a given dataset from start_time

Comment on lines +5 to +17
"properties": {
"timestamp": {
"type": "integer"
},
"thing": {
"type": "string"
},
"id": {
"type": ["null", "string"]
},
"airbyte_cursor": {
"type": "number"
}
Copy link
Member

Choose a reason for hiding this comment

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

This is correct to all possible datasets?

Copy link
Contributor Author

@andnig andnig Nov 8, 2022

Choose a reason for hiding this comment

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

This is the minimum set on columns available for all Senseforce datasets.
Senseforce users can than dynamically add additional columns but they are purely dependent on the users data model which again is fully flexibly defined by the user - so the connector does not know the full dataset in advance.
Long story short: The columns defined here are part of any Senseforce dataset as per the set up manual of this connector - but there most certainly are additional columns, which are dynamically added or removed from Senseforce users.

@andnig
Copy link
Contributor Author

andnig commented Nov 8, 2022

Some comments, do you have test account to share to run tests?

@marcosmarxm Removed the file and answered your comments.
I sent you the credentials for a sandbox dataset in Slack on the 1st of November. My Slack Name is "Andreas Nigg". Are these credentials sufficient?

@andnig andnig requested a review from marcosmarxm November 8, 2022 12:06
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

Thanks @andnig I'll publish your connector later!

andnig and others added 6 commits November 9, 2022 08:56
This commit is the initial commit to the senseforce.io source.
It contains the auto-generated lowcode CDK source connector files
as well as the spec definition of the connector.

Not working yet.
This commit adds the stream slicing and incremental sync feature.

This commit passes all configured acceptance tests.
@marcosmarxm
Copy link
Member

marcosmarxm commented Nov 9, 2022

/test connector=connectors/source-senseforce

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

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       133      3    98%   87, 93, 230
	 source_acceptance_test/conftest.py                     196     97    51%   35, 41-43, 48, 54, 60, 66, 72-74, 80-95, 100, 105-107, 113-115, 121-122, 127-128, 133, 139, 148-157, 163-168, 232, 238, 244-250, 258-263, 271-284, 289-295, 302-313, 320-336
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              329    106    68%   39, 50-58, 63-70, 74-75, 79-80, 164, 202-219, 228-236, 240-245, 251, 284-289, 327-334, 377-379, 382, 447-455, 484-485, 491, 494, 530-540, 553-578
	 source_acceptance_test/tests/test_incremental.py       145     20    86%   21-23, 29-31, 36-43, 48-61, 224
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  77     10    87%   15-16, 24-30, 64, 67
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/config_migration.py        23     23     0%   5-37
	 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                                                 1479    376    75%

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:51: The previous connector image could not be retrieved.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:229: The previous connector image could not be retrieved.
======================== 27 passed, 2 skipped in 50.69s ========================

@marcosmarxm
Copy link
Member

marcosmarxm commented Nov 9, 2022

/publish connector=connectors/source-senseforce

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


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

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

@marcosmarxm
Copy link
Member

marcosmarxm commented Nov 9, 2022

/publish connector=connectors/source-senseforce

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


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

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

@marcosmarxm
Copy link
Member

marcosmarxm commented Nov 9, 2022

/test connector=connectors/source-senseforce

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

	 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              345    110    68%   53, 64-72, 77-84, 88-89, 93-94, 178, 216-233, 242-250, 254-259, 265, 298-303, 341-348, 391-393, 396, 461-469, 481-484, 489, 545-546, 552, 555, 591-601, 614-639
	 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                                                 1508    348    77%

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:65: The previous connector image could not be retrieved.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:243: The previous connector image could not be retrieved.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_incremental.py:26: `future_state` not specified, skipping.
=================== 26 passed, 3 skipped in 71.65s (0:01:11) ===================

@marcosmarxm
Copy link
Member

marcosmarxm commented Nov 9, 2022

/publish connector=connectors/source-senseforce

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


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

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

@marcosmarxm marcosmarxm merged commit c8a00ea into airbytehq:master Nov 9, 2022
@sajarin sajarin changed the title 🎉 New Source: Senseforce 🎉 New Source: Senseforce [low-code CDK] Nov 9, 2022
akashkulk pushed a commit that referenced this pull request Dec 2, 2022
* feat: initial commit source senseforce

This commit is the initial commit to the senseforce.io source.
It contains the auto-generated lowcode CDK source connector files
as well as the spec definition of the connector.

Not working yet.

* feat: stream slicing and incremental sync [pass]

This commit adds the stream slicing and incremental sync feature.

This commit passes all configured acceptance tests.

* docs: Update documentation

* change: remove unused schema readme

* change: remove catalog.json from integration_tests

* add senseforce to source def

* run format

* correct tests

* auto-bump connector version

Co-authored-by: marcosmarxm <[email protected]>
Co-authored-by: Octavia Squidington III <[email protected]>
@sajarin sajarin added internal and removed bounty labels Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants