Source FreshCaller: fix spec type check #26065

artem1205 · 2023-05-15T09:17:04Z

What

Resolve #25688
Handle config errors for start_date
Start_date field was not declared as a string in jsonschema, but was rendered as a string in UI.
This may have caused a problem with validation (see screenshot from #25688)

How

Fix spec type for start_date

🚨 User Impact 🚨

no breaking changes

Pre-merge Actions

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Unit & integration tests added

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.

github-actions · 2023-05-15T09:17:23Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan and you've followed all steps in the Breaking Changes Checklist
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
You, or an Airbyter, have run /test successfully on this PR - or on a non-forked branch
You, or an Airbyter, have run /publish successfully on this PR - or on a non-forked branch
You've updated the connector's metadata.yaml file new!

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

artem1205 · 2023-05-15T09:55:57Z

/test connector=connectors/source-freshcaller

🕑 connectors/source-freshcaller https://github.com/airbytehq/airbyte/actions/runs/4978991998

artem1205 · 2023-05-15T10:29:06Z

/test connector=connectors/source-freshcaller

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

Name                             Stmts   Miss  Cover
----------------------------------------------------
source_freshcaller/__init__.py       2      0   100%
source_freshcaller/source.py        29     11    62%
source_freshcaller/streams.py      100     47    53%
----------------------------------------------------
TOTAL                              131     58    56%
Name                             Stmts   Miss  Cover
----------------------------------------------------
source_freshcaller/__init__.py       2      0   100%
source_freshcaller/streams.py      100      4    96%
source_freshcaller/source.py        29     15    48%
----------------------------------------------------
TOTAL                              131     19    85%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:109: Backward compatibility tests are disabled for version 0.1.0.
============ 39 passed, 1 skipped, 41 warnings in 142.78s (0:02:22) ============

alafanechere

Hey @artem1205 I'd appreciate a bit more context explaining why this change is solving the issue you linked and why it's backward incompatible?

alafanechere · 2023-05-15T14:45:40Z

airbyte-integrations/connectors/source-freshcaller/acceptance-test-config.yml

+      backward_compatibility_tests_config:
+        disable_for_version: 0.1.0 # Fix start_date type


Were the backward compatibility tests failing before this change? I'm not sure the change you made are actually backward incompatible? What was the default type stored for date before you specifically declare it's a string?

Start_date field was not declared as a string in jsonschema, but was rendered as a string in UI.
This possible caused a pattern validation error (see screenshot from #25688)
error without disabling:

alafanechere · 2023-05-15T14:46:11Z

airbyte-integrations/connectors/source-freshcaller/source_freshcaller/streams.py

@@ -126,15 +126,15 @@ def stream_slices(self, stream_state: Mapping[str, Any] = None, **kwargs) -> Ite
            ...]
        """
        start_date = pendulum.parse(self.start_date).in_timezone("UTC")
-        end_date = pendulum.utcnow().subtract(minutes=self.sync_lag_minutes)  # have a safe lag
+        end_date = pendulum.now("UTC").subtract(minutes=self.sync_lag_minutes)  # have a safe lag


Why is this change required?

I've bumped airbyte-cdk to ~=0.2 and removed pendulum in setup.py.
In Pendulum utcnow() is deprecated since 2.0.0

artem1205 · 2023-05-15T15:12:30Z

Hey @artem1205 I'd appreciate a bit more context explaining why this change is solving the issue you linked and why it's backward incompatible?

Description updated. It is potentially backward compatible, as there was not any info about the type in schema.
Maybe we need to add another check in our tests for this case, I mean , check that every field in our spec should have type property, WDYT @alafanechere ?

alafanechere

Thank you I think I now understand clearly the reason of the change. I hoped this issue to improve spec testing as you suggested

airbyte-integrations/connectors/source-freshcaller/Dockerfile

artem1205 · 2023-05-16T22:06:42Z

/publish connector=connectors/source-freshcaller

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

Connector	Version	Did it publish?	Were definitions generated?
connectors/source-freshcaller	0.2.0	✅	❌

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

* Source FreshCaller: fix spec type check * Source FreshCaller: update docs * Source FreshCaller: fix schemas * Source FreshCaller: reformat * Source FreshCaller: bump version minor * Source FreshCaller: update metadata

Source FreshCaller: fix spec type check

ef2de72

octavia-squidington-iii added area/connectors Connector related issues connectors/source/freshcaller labels May 15, 2023

Source FreshCaller: update docs

ec851c9

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label May 15, 2023

Source FreshCaller: fix schemas

a66b3c7

Source FreshCaller: reformat

041261a

artem1205 requested review from grubberr, lazebnyi and a team May 15, 2023 10:40

alafanechere requested changes May 15, 2023

View reviewed changes

alafanechere self-assigned this May 16, 2023

alafanechere approved these changes May 16, 2023

View reviewed changes

airbyte-integrations/connectors/source-freshcaller/Dockerfile Outdated Show resolved Hide resolved

Source FreshCaller: bump version minor

678c199

Source FreshCaller: update metadata

ff6e36f

artem1205 merged commit cf5cdd4 into master May 16, 2023
19 checks passed

artem1205 deleted the artem1205/source-freshcaller-config-errors-25688 branch May 16, 2023 22:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Source FreshCaller: fix spec type check #26065

Source FreshCaller: fix spec type check #26065

artem1205 commented May 15, 2023 •

edited

Loading

github-actions bot commented May 15, 2023 •

edited by artem1205

Loading

artem1205 commented May 15, 2023 •

edited by github-actions bot

Loading

artem1205 commented May 15, 2023 •

edited by github-actions bot

Loading

alafanechere left a comment

alafanechere May 15, 2023

artem1205 May 15, 2023

alafanechere May 15, 2023

artem1205 May 15, 2023

artem1205 commented May 15, 2023

alafanechere left a comment

artem1205 commented May 16, 2023 •

edited by github-actions bot

Loading

		backward_compatibility_tests_config:
		disable_for_version: 0.1.0 # Fix start_date type

Source FreshCaller: fix spec type check #26065

Source FreshCaller: fix spec type check #26065

Conversation

artem1205 commented May 15, 2023 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Actions

Community member or Airbyter

Airbyter

github-actions bot commented May 15, 2023 • edited by artem1205 Loading

Before Merging a Connector Pull Request

artem1205 commented May 15, 2023 • edited by github-actions bot Loading

artem1205 commented May 15, 2023 • edited by github-actions bot Loading

Build Passed

alafanechere left a comment

Choose a reason for hiding this comment

alafanechere May 15, 2023

Choose a reason for hiding this comment

artem1205 May 15, 2023

Choose a reason for hiding this comment

alafanechere May 15, 2023

Choose a reason for hiding this comment

artem1205 May 15, 2023

Choose a reason for hiding this comment

artem1205 commented May 15, 2023

alafanechere left a comment

Choose a reason for hiding this comment

artem1205 commented May 16, 2023 • edited by github-actions bot Loading

artem1205 commented May 15, 2023 •

edited

Loading

github-actions bot commented May 15, 2023 •

edited by artem1205

Loading

artem1205 commented May 15, 2023 •

edited by github-actions bot

Loading

artem1205 commented May 15, 2023 •

edited by github-actions bot

Loading

artem1205 commented May 16, 2023 •

edited by github-actions bot

Loading