🐛Source Harvest: “started_time” being incorrectly cast as datetime fi…

…eld (#15312) * Increased unit test coverage * Fixed schema format error * Updated PR number * Uncomment connection tests * Fixed typo * Fix request_params if stream_slice None * auto-bump connector version [ci skip] Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
airbytehq · Aug 8, 2022 · 43e7fbb · 43e7fbb
1 parent 8d9a3aa
commit 43e7fbb
Show file tree

Hide file tree

Showing 14 changed files with 218 additions and 101 deletions.
diff --git a/airbyte-config/init/src/main/resources/seed/source_definitions.yaml b/airbyte-config/init/src/main/resources/seed/source_definitions.yaml
@@ -399,7 +399,7 @@
 - name: Harvest
   sourceDefinitionId: fe2b4084-3386-4d3b-9ad6-308f61a6f1e6
   dockerRepository: airbyte/source-harvest
-  dockerImageTag: 0.1.8
+  dockerImageTag: 0.1.9
   documentationUrl: https://docs.airbyte.io/integrations/sources/harvest
   icon: harvest.svg
   sourceType: api

diff --git a/airbyte-config/init/src/main/resources/seed/source_specs.yaml b/airbyte-config/init/src/main/resources/seed/source_specs.yaml
@@ -3547,7 +3547,7 @@
     supportsNormalization: false
     supportsDBT: false
     supported_destination_sync_modes: []
-- dockerImage: "airbyte/source-harvest:0.1.8"
+- dockerImage: "airbyte/source-harvest:0.1.9"
   spec:
     documentationUrl: "https://docs.airbyte.io/integrations/sources/harvest"
     connectionSpecification:
@@ -3587,14 +3587,11 @@
             - "client_id"
             - "client_secret"
             - "refresh_token"
-            additionalProperties: false
+            additionalProperties: true
             properties:
               auth_type:
                 type: "string"
                 const: "Client"
-                enum:
-                - "Client"
-                default: "Client"
                 order: 0
               client_id:
                 title: "Client ID"
@@ -3614,14 +3611,11 @@
             title: "Authenticate with Personal Access Token"
             required:
             - "api_token"
-            additionalProperties: false
+            additionalProperties: true
             properties:
               auth_type:
                 type: "string"
                 const: "Token"
-                enum:
-                - "Token"
-                default: "Token"
                 order: 0
               api_token:
                 title: "Personal Access Token"
@@ -3654,7 +3648,7 @@
       oauth_config_specification:
         complete_oauth_output_specification:
           type: "object"
-          additionalProperties: false
+          additionalProperties: true
           properties:
             refresh_token:
               type: "string"
@@ -3663,15 +3657,15 @@
               - "refresh_token"
         complete_oauth_server_input_specification:
           type: "object"
-          additionalProperties: false
+          additionalProperties: true
           properties:
             client_id:
               type: "string"
             client_secret:
               type: "string"
         complete_oauth_server_output_specification:
           type: "object"
-          additionalProperties: false
+          additionalProperties: true
           properties:
             client_id:
               type: "string"

diff --git a/airbyte-integrations/connectors/source-harvest/Dockerfile b/airbyte-integrations/connectors/source-harvest/Dockerfile
@@ -12,5 +12,5 @@ RUN pip install .
 ENV AIRBYTE_ENTRYPOINT "python /airbyte/integration_code/main.py"
 ENTRYPOINT ["python", "/airbyte/integration_code/main.py"]
 
-LABEL io.airbyte.version=0.1.8
+LABEL io.airbyte.version=0.1.9
 LABEL io.airbyte.name=airbyte/source-harvest
diff --git a/airbyte-integrations/connectors/source-harvest/README.md b/airbyte-integrations/connectors/source-harvest/README.md
@@ -79,7 +79,7 @@ docker run --rm -v $(pwd)/secrets:/secrets -v $(pwd)/integration_tests:/integrat
 Make sure to familiarize yourself with [pytest test discovery](https://docs.pytest.org/en/latest/goodpractices.html#test-discovery) to know how your test files and methods should be named.
 First install test dependencies into your virtual environment:
 ```
-pip install .[tests]
+pip install .'[tests]'
 ```
 ### Unit Tests
 To run unit tests locally, from the connector directory run:

diff --git a/airbyte-integrations/connectors/source-harvest/integration_tests/acceptance.py b/airbyte-integrations/connectors/source-harvest/integration_tests/acceptance.py
@@ -11,6 +11,4 @@
 @pytest.fixture(scope="session", autouse=True)
 def connector_setup():
     """This fixture is a placeholder for external resources that acceptance test might require."""
-    # TODO: setup test dependencies if needed. otherwise remove the TODO comments
     yield
-    # TODO: clean up test dependencies
diff --git a/airbyte-integrations/connectors/source-harvest/requirements.txt b/airbyte-integrations/connectors/source-harvest/requirements.txt
@@ -0,0 +1,3 @@
+# This file is autogenerated -- only edit if you know what you are doing. Use setup.py for declaring dependencies.
+-e ../../bases/source-acceptance-test
+-e .
diff --git a/airbyte-integrations/connectors/source-harvest/setup.py b/airbyte-integrations/connectors/source-harvest/setup.py
@@ -11,6 +11,8 @@
 
 TEST_REQUIREMENTS = [
     "pytest~=6.1",
+    "requests-mock",
+    "source-acceptance-test",
 ]
 
 setup(

diff --git a/airbyte-integrations/connectors/source-harvest/source_harvest/schemas/time_entries.json b/airbyte-integrations/connectors/source-harvest/source_harvest/schemas/time_entries.json
@@ -151,12 +151,10 @@
       "type": ["null", "string"]
     },
     "started_time": {
-      "type": ["null", "string"],
-      "format": "date-time"
+      "type": ["null", "string"]
     },
     "ended_time": {
-      "type": ["null", "string"],
-      "format": "date-time"
+      "type": ["null", "string"]
     },
     "is_running": {
       "type": ["null", "boolean"]

diff --git a/airbyte-integrations/connectors/source-harvest/source_harvest/spec.json b/airbyte-integrations/connectors/source-harvest/source_harvest/spec.json
@@ -32,13 +32,11 @@
             "type": "object",
             "title": "Authenticate via Harvest (OAuth)",
             "required": ["client_id", "client_secret", "refresh_token"],
-            "additionalProperties": false,
+            "additionalProperties": true,
             "properties": {
               "auth_type": {
                 "type": "string",
                 "const": "Client",
-                "enum": ["Client"],
-                "default": "Client",
                 "order": 0
               },
               "client_id": {
@@ -64,13 +62,11 @@
             "type": "object",
             "title": "Authenticate with Personal Access Token",
             "required": ["api_token"],
-            "additionalProperties": false,
+            "additionalProperties": true,
             "properties": {
               "auth_type": {
                 "type": "string",
                 "const": "Token",
-                "enum": ["Token"],
-                "default": "Token",
                 "order": 0
               },
               "api_token": {
@@ -102,7 +98,7 @@
     "oauth_config_specification": {
       "complete_oauth_output_specification": {
         "type": "object",
-        "additionalProperties": false,
+        "additionalProperties": true,
         "properties": {
           "refresh_token": {
             "type": "string",
@@ -112,7 +108,7 @@
       },
       "complete_oauth_server_input_specification": {
         "type": "object",
-        "additionalProperties": false,
+        "additionalProperties": true,
         "properties": {
           "client_id": {
             "type": "string"
@@ -124,7 +120,7 @@
       },
       "complete_oauth_server_output_specification": {
         "type": "object",
-        "additionalProperties": false,
+        "additionalProperties": true,
         "properties": {
           "client_id": {
             "type": "string",

diff --git a/airbyte-integrations/connectors/source-harvest/source_harvest/streams.py b/airbyte-integrations/connectors/source-harvest/source_harvest/streams.py
@@ -24,6 +24,13 @@ def data_field(self) -> str:
         """
         return self.name
 
+    def backoff_time(self, response: requests.Response):
+        if "Retry-After" in response.headers:
+            return int(response.headers["Retry-After"])
+        else:
+            self.logger.info("Retry-after header not found. Using default backoff value")
+            return super().backoff_time(response)
+
     def path(self, **kwargs) -> str:
         return self.name
 
@@ -294,14 +301,6 @@ def __init__(self, from_date: pendulum.date = None, **kwargs):
         else:
             self._to_date = current_date
 
-    def request_params(self, stream_state, **kwargs) -> MutableMapping[str, Any]:
-        params = super().request_params(stream_state, **kwargs)
-        current_date = pendulum.now()
-        # `from` and `to` params are required for reports calls
-        # min `from` value is current_date - 1 year
-        params.update({"from": self._from_date.strftime("%Y%m%d"), "to": current_date.strftime("%Y%m%d")})
-        return params
-
     def path(self, **kwargs) -> str:
         return f"reports/{self.report_path}"
 
@@ -323,26 +322,9 @@ def parse_response(self, response: requests.Response, **kwargs) -> Iterable[Mapp
             )
             yield record
 
-    def request_params(self, stream_state: Mapping[str, Any] = None, **kwargs) -> MutableMapping[str, Any]:
-        stream_state = stream_state or {}
+    def request_params(self, stream_state: Mapping[str, Any], stream_slice: Mapping[str, Any] = None, **kwargs) -> MutableMapping[str, Any]:
         params = super().request_params(stream_state, **kwargs)
-
-        # subtract `from` date by 1 year to avoid Harvest exception
-        # `from` date may not be less than `to` - 1 year
-        if stream_state.get(self.cursor_field):
-            cursor_date = pendulum.parse(stream_state[self.cursor_field]).date()
-            dates_diff = cursor_date - self._from_date
-            if dates_diff.years > 0 or dates_diff.years == 0 and dates_diff.remaining_days > 0:
-                self._from_date = cursor_date.subtract(years=1)
-
-        # `from` and `to` params are required for reports calls
-        # min `from` value is current_date - 1 year
-        params.update(
-            {
-                "from": self._from_date.strftime(self.date_param_template),
-                "to": stream_state.get(self.cursor_field, self._to_date.strftime(self.date_param_template)),
-            }
-        )
+        params = {**params, **stream_slice} if stream_slice else params
         return params
 
     def get_updated_state(self, current_stream_state: MutableMapping[str, Any], latest_record: Mapping[str, Any]):
@@ -355,6 +337,27 @@ def get_updated_state(self, current_stream_state: MutableMapping[str, Any], late
             return {self.cursor_field: max(latest_benchmark, current_stream_state[self.cursor_field])}
         return {self.cursor_field: latest_benchmark}
 
+    def stream_slices(self, sync_mode, stream_state: Mapping[str, Any] = None, **kwargs) -> Iterable[Optional[MutableMapping[str, any]]]:
+        """
+        Override default stream_slices CDK method to provide date_slices as page chunks for data fetch.
+        """
+        start_date = self._from_date
+        end_date = pendulum.now().date()
+
+        # determine stream_state, if no stream_state we use start_date
+        if stream_state:
+            start_date = pendulum.parse(stream_state.get(self.cursor_field)).date()
+
+        while start_date < end_date:
+            # Max size of date chunks is 1 year
+            # Docs: https://help.getharvest.com/api-v2/reports-api/reports/time-reports/
+            end_date_slice = end_date if start_date >= end_date.subtract(years=1) else start_date.add(years=1)
+            date_slice = {"from": start_date.strftime(self.date_param_template), "to": end_date_slice.strftime(self.date_param_template)}
+
+            start_date = end_date_slice
+
+            yield date_slice
+
 
 class ExpensesClients(IncrementalReportsBase):
     """
@@ -388,11 +391,9 @@ class ExpensesTeam(IncrementalReportsBase):
     report_path = "expenses/team"
 
 
-class Uninvoiced(ReportsBase):
+class Uninvoiced(IncrementalReportsBase):
     """
     Docs: https://help.getharvest.com/api-v2/reports-api/reports/uninvoiced-report/
-
-    TODO: `from`/`to` pagination does not work for `uninvoiced` stream. Look like a bug on Harvest side. Check out later.
     """
 
     report_path = "uninvoiced"
@@ -424,15 +425,15 @@ class TimeTasks(IncrementalReportsBase):
 
 class TimeTeam(IncrementalReportsBase):
     """
-    Docs: https://help.getharvest.com/api-v2/reports-api/reports/time-reports/ (Team Report)
+    Docs: https://help.getharvest.com/api-v2/reports-api/reports/time-reports/
     """
 
     report_path = "time/team"
 
 
 class ProjectBudget(ReportsBase):
     """
-    Docs: https://help.getharvest.com/api-v2/reports-api/reports/time-reports/#team-report
+    Docs: https://help.getharvest.com/api-v2/reports-api/reports/project-budget-report/#project-budget-report
     """
 
     report_path = "project_budget"
diff --git a/airbyte-integrations/connectors/source-harvest/unit_tests/conftest.py b/airbyte-integrations/connectors/source-harvest/unit_tests/conftest.py
@@ -0,0 +1,35 @@
+#
+# Copyright (c) 2022 Airbyte, Inc., all rights reserved.
+#
+
+from pendulum import parse
+from pytest import fixture
+
+
+@fixture(name="config")
+def config_fixture(requests_mock):
+    url = "https://id.getharvest.com/api/v2/oauth2/token"
+    requests_mock.get(url, json={})
+
+    config = {"account_id": "ID", "replication_start_date": "2021-01-01T21:20:07Z", "credentials": {"api_token": "TOKEN"}}
+
+    return config
+
+
+@fixture(name="replication_start_date")
+def replication_start_date_fixture(config):
+    return parse(config["replication_start_date"])
+
+
+@fixture(name="from_date")
+def from_date_fixture(replication_start_date):
+    return replication_start_date.date()
+
+
+@fixture(name="mock_stream")
+def mock_stream_fixture(requests_mock):
+    def _mock_stream(path, response={}):
+        url = f"https://api.harvestapp.com/v2/{path}"
+        requests_mock.get(url, json=response)
+
+    return _mock_stream