Skip to content

Facebook marketing: Fix videos stream invalid type on jsonschema - #9138

Merged
Dmytro (avida) merged 3 commits into
masterfrom
drezchykov/fb-videos-schema-fix
Dec 29, 2021
Merged

Facebook marketing: Fix videos stream invalid type on jsonschema#9138
Dmytro (avida) merged 3 commits into
masterfrom
drezchykov/fb-videos-schema-fix

Conversation

@avida

@avida Dmytro (avida) commented Dec 28, 2021

Copy link
Copy Markdown
Contributor

Resolves #8975

@github-actions github-actions Bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Dec 28, 2021
@avida

Dmytro (avida) commented Dec 28, 2021 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor Author

/test connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/1629901676
❌ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/1629901676
🐛 https://gradle.com/s/yuikt6pofuacu

@avida
Dmytro (avida) temporarily deployed to more-secrets December 28, 2021 09:10 Inactive
@jrhizor
Jared Rhizor (jrhizor) temporarily deployed to more-secrets December 28, 2021 09:12 Inactive
@avida

Dmytro (avida) commented Dec 28, 2021 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor Author

/test connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/1630410035
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/1630410035
Python tests coverage:

	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                                 Stmts   Miss  Cover
	 ------------------------------------------------------------------------
	 source_acceptance_test/__init__.py                       2      0   100%
	 source_acceptance_test/base.py                          10      4    60%
	 source_acceptance_test/config.py                        74      6    92%
	 source_acceptance_test/conftest.py                     109    109     0%
	 source_acceptance_test/plugin.py                        47     47     0%
	 source_acceptance_test/tests/__init__.py                 4      0   100%
	 source_acceptance_test/tests/test_core.py              242     96    60%
	 source_acceptance_test/tests/test_full_refresh.py       38      0   100%
	 source_acceptance_test/tests/test_incremental.py        69     38    45%
	 source_acceptance_test/utils/__init__.py                 6      0   100%
	 source_acceptance_test/utils/asserts.py                 37      2    95%
	 source_acceptance_test/utils/common.py                  54     17    69%
	 source_acceptance_test/utils/compare.py                 62     23    63%
	 source_acceptance_test/utils/connector_runner.py       110     48    56%
	 source_acceptance_test/utils/json_schema_helper.py     115     14    88%
	 ------------------------------------------------------------------------
	 TOTAL                                                  979    404    59%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                     Stmts   Miss  Cover
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/.venv/lib/python3.8/site-packages/coverage/report.py:87: CoverageWarning: Couldn't parse '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-31>': No source for code: '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-31>'. (couldnt-parse)
	   coverage._warn(msg, slug="couldnt-parse")
	 /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/.venv/lib/python3.8/site-packages/coverage/report.py:87: CoverageWarning: Couldn't parse '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-32>': No source for code: '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-32>'. (couldnt-parse)
	   coverage._warn(msg, slug="couldnt-parse")
	 /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/.venv/lib/python3.8/site-packages/coverage/report.py:87: CoverageWarning: Couldn't parse '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-33>': No source for code: '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-33>'. (couldnt-parse)
	   coverage._warn(msg, slug="couldnt-parse")
	 ------------------------------------------------------------
	 /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/.venv/lib/python3.8/site-packages/coverage/report.py:87: CoverageWarning: Couldn't parse '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-34>': No source for code: '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-34>'. (couldnt-parse)
	   coverage._warn(msg, slug="couldnt-parse")
	 source_facebook_marketing/__init__.py        2      0   100%
	 source_facebook_marketing/api.py            75     17    77%
	 source_facebook_marketing/async_job.py      92     58    37%
	 source_facebook_marketing/common.py         37     11    70%
	 source_facebook_marketing/source.py        114     66    42%
	 source_facebook_marketing/streams.py       253     90    64%
	 ------------------------------------------------------------
	 /actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/.venv/lib/python3.8/site-packages/coverage/report.py:87: CoverageWarning: Couldn't parse '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-35>': No source for code: '/actions-runner/_work/airbyte/airbyte/airbyte-integrations/connectors/source-facebook-marketing/rep-35>'. (couldnt-parse)
	   coverage._warn(msg, slug="couldnt-parse")
	 TOTAL                                      573    242    58%
	 ---------- coverage: platform linux, python 3.8.10-final-0 -----------
	 Name                                     Stmts   Miss  Cover
	 ------------------------------------------------------------
	 source_facebook_marketing/__init__.py        2      0   100%
	 source_facebook_marketing/api.py            75     18    76%
	 source_facebook_marketing/async_job.py      92      1    99%
	 source_facebook_marketing/common.py         37      1    97%
	 source_facebook_marketing/source.py        114     73    36%
	 source_facebook_marketing/streams.py       253     89    65%
	 ------------------------------------------------------------
	 TOTAL                                      573    182    68%

@avida
Dmytro (avida) temporarily deployed to more-secrets December 28, 2021 12:11 Inactive
@jrhizor
Jared Rhizor (jrhizor) temporarily deployed to more-secrets December 28, 2021 12:11 Inactive
avida
Dmytro (avida) commented Dec 28, 2021
View reviewed changes
Comment thread airbyte-integrations/connectors/source-facebook-marketing/integration_tests/test_streams.py
assert is_specific_deleted_pulled, f"{stream_name} stream should have a deleted record with id={deleted_id}"

@pytest.mark.parametrize("stream_name, deleted_num", [("ads", 2), ("campaigns", 3), ("ad_sets", 1)])
@pytest.mark.parametrize("stream_name, deleted_num", [("ads", 2), ("campaigns", 1), ("ad_sets", 1)])

@avida Dmytro (avida) Dec 28, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it was correct to update number of deleted campaigns. Could it be related to start_date parameter change in the config or this is a bug Eugene Kulak (@keu) ?

yevhenii-ldv
Yevhenii (yevhenii-ldv) approved these changes Dec 28, 2021
View reviewed changes

@yevhenii-ldv Yevhenii (yevhenii-ldv) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, in general! Except, not sure if the tests need to be updated. Why could they break?

yevhenii-ldv
Yevhenii (yevhenii-ldv) reviewed Dec 28, 2021
View reviewed changes
Comment thread airbyte-config/init/src/main/resources/seed/source_definitions.yaml
sourceDefinitionId: e7778cfc-e97c-4458-9ecb-b4f2bba8946c
dockerRepository: airbyte/source-facebook-marketing
dockerImageTag: 0.2.30
dockerImageTag: 0.2.31

@yevhenii-ldv Yevhenii (yevhenii-ldv) Dec 28, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This parameter should be changed after publishing, because pipeline crashes

@avida

Dmytro (avida) commented Dec 29, 2021 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor Author

/publish connector=connectors/source-facebook-marketing

🕑 connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/1633399942
✅ connectors/source-facebook-marketing https://github.com/airbytehq/airbyte/actions/runs/1633399942

@jrhizor
Jared Rhizor (jrhizor) temporarily deployed to more-secrets December 29, 2021 08:54 Inactive
@avida
Dmytro (avida) merged commit 9da419d into master Dec 29, 2021
@avida
Dmytro (avida) deleted the drezchykov/fb-videos-schema-fix branch December 29, 2021 09:03
@avida
Dmytro (avida) temporarily deployed to more-secrets December 29, 2021 09:04 Inactive
@jrhizor Jared Rhizor (jrhizor) mentioned this pull request Jan 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sergei-solonitcyn Sergei Solonitcyn (sergei-solonitcyn) Awaiting requested review from sergei-solonitcyn
@Zirochkaa oleh.zorenko (Zirochkaa) Awaiting requested review from Zirochkaa
1 more reviewer
@yevhenii-ldv Yevhenii (yevhenii-ldv) yevhenii-ldv approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/facebook-marketing connectors/sources-api

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Source Facebook: uses Invalid JSON Schema in video stream

4 participants

@avida @yevhenii-ldv @jrhizor @karinakuz