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

source-delighted: Unix changed to normal timestamp #13439

Merged
merged 11 commits into from
Jun 10, 2022

Conversation

michaelnguyen26
Copy link
Contributor

What

Improved the source-delighted page by allowing users to enter a normal timestamp instead of a unix timestamp.

How

Made changes to the spec.json and source.py under the source-delighted. The backend side still converts the timestamp to unix, but the frontend allows the user to enter a regular timestamp.

Recommended reading order

  1. spec.json
  2. source.py

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

./gradlew :airbyte-config:init:processResources

@github-actions github-actions bot added the area/connectors Connector related issues label Jun 2, 2022
@alafanechere alafanechere added the bounty-S Maintainer program: claimable small bounty PR label Jun 3, 2022
@alafanechere
Copy link
Contributor

Thank you for this contribution @michaelnguyen26 . I will run the tests and go for a first review.

@alafanechere
Copy link
Contributor

alafanechere commented Jun 3, 2022

/test connector=connectors/source-delighted

🕑 connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2433347858
❌ connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2433347858
🐛 https://gradle.com/s/7arljiviqxzeq

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestSpec::test_config_match_spec[inputs0] - Failed: Conf...
FAILED test_core.py::TestConnection::test_check[inputs0] - docker.errors.Cont...
FAILED test_core.py::TestConnection::test_check[inputs1] - docker.errors.Cont...
FAILED test_core.py::TestDiscovery::test_discover[inputs0] - docker.errors.Co...
ERROR test_core.py::TestDiscovery::test_defined_cursors_exist_in_schema[inputs0]
ERROR test_core.py::TestDiscovery::test_defined_refs_exist_in_schema[inputs0]
ERROR test_core.py::TestDiscovery::test_defined_keyword_exist_in_schema[inputs0-allOf]
ERROR test_core.py::TestDiscovery::test_defined_keyword_exist_in_schema[inputs0-not]
ERROR test_core.py::TestDiscovery::test_primary_keys_exist_in_schema[inputs0]
ERROR test_core.py::TestBasicRead::test_read[inputs0] - docker.errors.Contain...
ERROR test_full_refresh.py::TestFullRefresh::test_sequential_reads[inputs0]
ERROR test_incremental.py::TestIncremental::test_two_sequential_reads[inputs0]
ERROR test_incremental.py::TestIncremental::test_read_sequential_slices[inputs0]
ERROR test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
=================== 4 failed, 10 passed, 10 errors in 10.13s ===================

🕑 connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2433347858
❌ connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2433347858
🐛 https://gradle.com/s/5mmfdkxthvv4k

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 ERROR unit_tests/unit_test.py - AttributeError: 'function' object has no attr...
	 !!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
	 �[31m=============================== �[31m�[1m1 error�[0m�[31m in 0.62s�[0m�[31m ===============================�[0m

🕑 connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2433347858
❌ connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2433347858
🐛 https://gradle.com/s/sl4hmgjjjiqs4

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 FAILED unit_tests/unit_test.py::test_not_output_records_where_cursor_field_equals_state[Bounces-https:/api.delighted.com/v1/bounces.json-\n[\n    {"person_id": "1046789984", "email": "foo_test204@airbyte.io", "name": "Foo Test204", "bounced_at": 1641455286},\n    {"person_id": "1046789989", "email": "foo_test205@airbyte.io", "name": "Foo Test205", "bounced_at": 1641455286}\n]\n]
	 FAILED unit_tests/unit_test.py::test_not_output_records_where_cursor_field_equals_state[People-https:/api.delighted.com/v1/people.json-\n[\n    {"id": "1046789989", "name": "Foo Test205", "email": "foo_test205@airbyte.io", "created_at": 1641455285, "last_sent_at": 1641455285, "last_responded_at": null, "next_survey_scheduled_at": null}\n]\n]
	 FAILED unit_tests/unit_test.py::test_not_output_records_where_cursor_field_equals_state[SurveyResponses-https:/api.delighted.com/v1/survey_responses.json-\n[\n    {"id": "210554887", "person": "1042205953", "survey_type": "nps", "score": 0, "comment": "Test Comment202", "permalink": "https:/app.delighted.com/r/0q7QEdWzosv5G5c3w9gakivDwEIM5Hq0", "created_at": 1641289816, "updated_at": 1641289816, "person_properties": null, "notes": [], "tags": [], "additional_answers": []},\n    {"id": "210554885", "person": "1042205947", "survey_type": "nps", "score": 5, "comment": "Test Comment201", "permalink": "https:/app.delighted.com/r/GhWWrBT2wayswOc0AfT7fxpM3UwSpitN", "created_at": 1641289816, "updated_at": 1641289816, "person_properties": null, "notes": [], "tags": [], "additional_answers": []}\n]\n]
	 FAILED unit_tests/unit_test.py::test_not_output_records_where_cursor_field_equals_state[Unsubscribes-https:/api.delighted.com/v1/unsubscribes.json-\n[\n    {"person_id": "1040826319", "email": "foo_test64@airbyte.io", "name": "Foo Test64", "unsubscribed_at": 1641289584}\n]\n]
	 �[31m=================== �[31m�[1m4 failed�[0m, �[32m1 passed�[0m, �[33m16 warnings�[0m�[31m in 0.67s�[0m�[31m ===================�[0m

@alafanechere alafanechere removed bounty bounty-S Maintainer program: claimable small bounty PR labels Jun 3, 2022
@alafanechere alafanechere self-assigned this Jun 3, 2022
@alafanechere
Copy link
Contributor

The test failure is normal as I would need to update the config for this connector in our CI following your changes. I'll update the config and re-run the tests once you get back to me following my suggestions.

@alafanechere
Copy link
Contributor

Requesting review from @misteryeo as spec.json was modified.

@alafanechere
Copy link
Contributor

@michaelnguyen26 could you please give feedback after my suggestion so that we can make this PR move forward? 🙏

michaelnguyen26 and others added 3 commits June 9, 2022 16:16
…ted/source.py

Co-authored-by: Augustin <augustin@airbyte.io>
…ted/source.py

Co-authored-by: Augustin <augustin@airbyte.io>
…ted/spec.json

Co-authored-by: Augustin <augustin@airbyte.io>
@michaelnguyen26
Copy link
Contributor Author

@alafanechere sorry for the late response. I am still fresh and new with AirByte's source code and dealing with a large open source project. Please bear with me. The suggestions you've made makes sense to me and I do not see anything wrong with them. I merged them in. Thanks!

@alafanechere
Copy link
Contributor

alafanechere commented Jun 10, 2022

/test connector=connectors/source-delighted

🕑 connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2473995493
❌ connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2473995493
🐛 https://gradle.com/s/lt2zuad7cx6cg

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 ERROR unit_tests/unit_test.py - AttributeError: 'function' object has no attr...
	 !!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
	 �[31m=============================== �[31m�[1m1 error�[0m�[31m in 0.61s�[0m�[31m ===============================�[0m

@alafanechere
Copy link
Contributor

alafanechere commented Jun 10, 2022

/publish connector=connectors/source-delighted

🕑 connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2475297242
❌ Failed to publish connectors/source-delighted
❌ Couldn't auto-bump version for connectors/source-delighted
🕑 connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2475297242
🚀 Successfully published connectors/source-delighted
🚀 Auto-bumped version for connectors/source-delighted
✅ connectors/source-delighted https://github.com/airbytehq/airbyte/actions/runs/2475297242

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Jun 10, 2022
Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

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

@michaelnguyen26 I made the final changes required to fully implement my suggestion. Ready to merge. Thank you for the initiative!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation community connectors/source/delighted internal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants