🐛 Source Facebook Marketing: fix url parsing and add report that exposes conversions.

[manavkohli]

What

This solves the issue of being able to fetch conversion data from the Facebook Marketing API. The discussion can be found here: #5190

How

This pull request creates a new report that fetches the action_type (where omni_purchase) represents a conversion whereas the other reports cannot due to misconfigured breakdowns

Recommended reading order

1. x.java
2. y.python

Pre-merge Checklist

Community member or Airbyter

• 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
  • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
• PR name follows PR naming conventions
• Connector version bumped like described here

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
• Credentials added to Github CI. Instructions.
• /test connector=connectors/<name> command is passing.
• New Connector version released on Dockerhub by running the /publish command described here

[sherifnada]

@manavkohli thanks! will review it soon

[manavkohli]

Awesome, thanks!

[manavkohli]

@sherifnada @vitaliizazmic any feedback here?

[manavkohli]

Test results:

./gradlew :airbyte-integrations:connectors:source-facebook-marketing:unitTest
**         =============================== warnings summary ===============================
         .venv/lib/python3.9/site-packages/jsonschema/compat.py:6
         .venv/lib/python3.9/site-packages/jsonschema/compat.py:6
           /Users/manavkohli/dev/airbyte/airbyte-integrations/connectors/source-facebook-marketing/.venv/lib/python3.9/site-packages/jsonschema/compat.py:6: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.10 it will stop working
             from collections import MutableMapping, Sequence  # noqa
         
         -- Docs: https://docs.pytest.org/en/stable/warnings.html
         ======================== 6 passed, 2 warnings in 15.28s ========================
**

@sherifnada do I need a particular pair of keys to test the integration tests? They're failing for me on master.

[sherifnada]

@manavkohli to run those tests you need to add credentials for your connector in airbyte-integrations/connectors/source-facebook-marketing/secrets/config.json - would you be able to do that?

[sherifnada]

LGTM! one question on unit tests

[sherifnada]

would you mind adding a unit test for this method?

[manavkohli]

Yes definitely!

[manavkohli]

@sherifnada I actually did, but it looks like the tests are written to assert for a specific account, e.g:

  @pytest.mark.parametrize("stream_name, deleted_num", [("ads", 2), ("campaigns", 3), ("ad_sets", 1)])
             def test_streams_with_include_deleted_and_state(self, stream_name, deleted_num, config_with_include_deleted, configured_catalog, state):
                 """Should ignore state because of include_deleted enabled"""
                 catalog = self.slice_catalog(configured_catalog, {stream_name})
                 records, states = self._read_records(config_with_include_deleted, catalog, state=state)
                 deleted_records = list(filter(self._deleted_record, records))
             
         >       assert len(deleted_records) == deleted_num, f"{stream_name} should have {deleted_num} deleted records returned"
         E       AssertionError: ads should have 2 deleted records returned
         E       assert 10 == 2
         E         +10
         E         -2

@pytest.mark.parametrize(
                 "stream_name, deleted_id", [("ads", "23846756820320398"), ("campaigns", "23846541919710398"), ("ad_sets", "23846541706990398")]
             )
             def test_streams_with_include_deleted(self, stream_name, deleted_id, config_with_include_deleted, configured_catalog):
                 catalog = self.slice_catalog(configured_catalog, {stream_name})
                 records, states = self._read_records(config_with_include_deleted, catalog)
                 deleted_records = list(filter(self._deleted_record, records))
                 is_specific_deleted_pulled = deleted_id in list(map(self._object_id, records))
             
                 assert states, "incremental read should produce states"
                 for name, state in states[-1].state.data.items():
                     assert "include_deleted" in state, f"State for {name} should include `include_deleted` flag"
             
                 assert deleted_records, f"{stream_name} stream should have deleted records returned"
         >       assert is_specific_deleted_pulled, f"{stream_name} stream should have a deleted record with id={deleted_id}"
         E       AssertionError: ad_sets stream should have a deleted record with id=23846541706990398
         E       assert False

It looks like both of those tests are dependent on data from a particular account?

[manavkohli]

Added test coverage results:

         =============================== warnings summary ===============================
         .venv/lib/python3.9/site-packages/jsonschema/compat.py:6
         .venv/lib/python3.9/site-packages/jsonschema/compat.py:6
           /Users/manavkohli/dev/airbyte/airbyte-integrations/connectors/source-facebook-marketing/.venv/lib/python3.9/site-packages/jsonschema/compat.py:6: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated since Python 3.3, and in 3.10 it will stop working
             from collections import MutableMapping, Sequence  # noqa
         
         -- Docs: https://docs.pytest.org/en/stable/warnings.html
         ======================= 14 passed, 2 warnings in 15.26s ========================

[sherifnada]

It looks like both of those tests are dependent on data from a particular account?

Ah, it's a hack we've done in the test for FB to get around some limitations of our test infra. If you change that ID to the ID of your ad account do the tests pass? if so could you post a screenshot of the passing tests? we should be good to go aftewards

[manavkohli]

@sherifnada anything else needed here?

[sherifnada]

@manavkohli we've been debugging some test failures on our side. I'm pretty sure they should be non-blocking for your PR so I just published the connector and @vitaliizazmic will create an issue to follow up on the test failures. Thanks for your contribution!