Addressed review comments

Author: MuralidharT03

.circleci/config.yml

@@ -21,7 +21,7 @@ jobs:
           name: 'pylint'
           command: |
             source /usr/local/share/virtualenvs/tap-trello/bin/activate
-            pylint tap_trello --disable 'broad-except,chained-comparison,empty-docstring,fixme,invalid-name,line-too-long,missing-class-docstring,missing-function-docstring,missing-module-docstring,no-else-raise,no-else-return,too-few-public-methods,too-many-arguments,too-many-branches,too-many-lines,too-many-locals,ungrouped-imports,wrong-spelling-in-comment,wrong-spelling-in-docstring,consider-using-f-string,unspecified-encoding,broad-exception-raised,use-yield-from,logging-format-interpolation,unnecessary-pass,too-many-positional-arguments,too-many-statements'
+            pylint tap_trello -d C,R,W
       - run:
           name: 'Unit Tests'
           command: |

tap_trello/schemas/card_custom_field_items.json

@@ -62,6 +62,5 @@
                 "string"
             ]
         }
-    },
-    "required": ["id", "card_id"]
+    }
 }

tap_trello/streams/abstracts.py

@@ -11,28 +11,26 @@
 
 LOGGER = get_logger()
 
+
 class OrderChecker:
     """ Class with context manager to check ordering of values. """
     order = None
     _last_value = None
+    check_paired_order = None
 
     def __init__(self, order='ASC'):
         self.order = order
+        self.check_paired_order = operator.lt if order == 'ASC' else operator.gt
 
     def check_order(self, current_value):
         """
         We are sub-paginating based on a sort order descending assumption for
         Actions, this ensures that this holds up.
         """
-        if self.order == 'ASC':
-            check_paired_order = operator.lt
-        else:
-            check_paired_order = operator.gt
-
         if self._last_value is None:
             self._last_value = current_value
 
-        if check_paired_order(current_value, self._last_value):
+        if self.check_paired_order(current_value, self._last_value):
             asc_desc = "ascending" if self.order == 'ASC' else "descending"
             gt_lt = "less than" if self.order == 'ASC' else "greater than"
             raise Exception(
@@ -57,9 +55,11 @@ def __enter__(self):
     def __exit__(self, *args):
         """ Required for context manager usage. """
 
+
 class Mixin:
     """ Empty class to mark mixin classes as such. """
 
+
 class Unsortable(Mixin):
     """
     Mixin class to mark a Stream subclass as Unsortable
@@ -73,6 +73,7 @@ class MyStream(Unsortable, Stream):
 # NB: We've observed that Trello will only return 50 actions, this is to sub-paginate
 MAX_API_RESPONSE_SIZE = 50
 
+
 class DateWindowPaginated(Mixin):
     """
     Mixin class to provide date windowing on the `get_records` requests
@@ -171,7 +172,6 @@ def _paginate_window(self, window_start, window_end, format_values):
                 break
 
 
-
 class LegacyStream:
     """
     Legacy base class for Trello streams (pre-refactor).
@@ -240,6 +240,7 @@ def sync(self):
         for rec in self.get_records(self.get_format_values()):
             yield rec
 
+
 class AddCustomFields(Mixin):
     def _get_dropdown_option_key(self, field_id, option_id):
         return field_id + '_' + option_id
@@ -274,7 +275,6 @@ def modify_record(self, record, **kwargs):
         return record
 
 
-
 class AddBoardId(Mixin):
     def modify_record(self, record, **kwargs):
         board_id_list = kwargs['parent_id_list']
@@ -423,7 +423,6 @@ def sync(
          - https://github.com/singer-io/getting-started/blob/master/docs/SYNC_MODE.md
         """
 
-
     def get_records(self) -> Iterator:
         """Interacts with api client interaction and pagination."""
         self.params["page"] = self.page_size

tap_trello/streams/cards.py

@@ -17,8 +17,8 @@ class Cards(AddCustomFields, ChildStream):
 
     def get_records(self, format_values, additional_params=None):
         # Get max_api_response_size from config and set to parameter
-        cards_response_size = self.config.get('cards_response_size')
-        self.MAX_API_RESPONSE_SIZE = int(cards_response_size) if cards_response_size else self.MAX_API_RESPONSE_SIZE
+        cards_response_size = int(self.config.get('cards_response_size') or self.MAX_API_RESPONSE_SIZE)
+        self.MAX_API_RESPONSE_SIZE = min(cards_response_size, 1000)
         self.params = {'limit': self.MAX_API_RESPONSE_SIZE, 'customFieldItems': 'true'}
 
         # Set window_end with current time
@@ -27,21 +27,14 @@ def get_records(self, format_values, additional_params=None):
         # Build custom fields and dropdown object map for the specific parent
         custom_fields_map, dropdown_options_map = self.build_custom_fields_maps(parent_id_list=format_values)
 
-        while True:
+        has_more_pages = True
+        while has_more_pages:
 
             # Get records for cards before specified time
             # Reference: https://developer.atlassian.com/cloud/trello/guides/rest-api/api-introduction/#paging
             records = self.client.get(self._format_endpoint(format_values), params={"before": window_end,
                                                                                    **self.params})
 
-            # Raise exception if API returns more data than specified limit
-            if self.MAX_API_RESPONSE_SIZE and len(records) > self.MAX_API_RESPONSE_SIZE:
-                raise Exception(
-                    ("{}: Number of records returned is greater than the requested API response size of {}.").format(
-                        self.stream_id,
-                        self.MAX_API_RESPONSE_SIZE)
-                )
-
             # Yielding records after adding custom fields and dropdown object map to all records
             for rec in records:
                 yield self.modify_record(rec, parent_id_list = format_values, custom_fields_map = custom_fields_map, dropdown_options_map = dropdown_options_map)
@@ -58,5 +51,5 @@ def get_records(self, format_values, additional_params=None):
                 # API returns latest records so set window_end to smallest card id to get older data
                 window_end = records[0]["id"]
             else:
-                # API returns less records than limit, break the pagination
-                break
+                # API returns less records than limit, stop pagination
+                has_more_pages = False